CAN status and tasks stuff by changxu-liu · Pull Request #13 · lhr-solar/PS-PowerDistributionUnit

changxu-liu · 2026-06-25T06:53:15Z

No description provided.

gemini-code-assist

Code Review

This pull request modularizes the Power Distribution Unit (PDU) Mk1 firmware by splitting tasks into dedicated files under tasks/ and adding test suites under test/, alongside introducing CAN status transmission. Several critical issues were identified in the review: FreeRTOS task functions must be updated to accept a void * parameter to match the expected prototype and avoid undefined behavior; an unaligned memory access in Task_CANSendStatus.c should be resolved using byte shifting; and the hfdcan3 pointer must be properly declared/initialized before dereferencing. Additionally, non-standard emojis in macro names should be replaced with ASCII, copy-paste task names in tests should be corrected, and a redundant loop around Error_Handler() should be cleaned up.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-25T06:55:10Z

+void Task_ReadCurrents()
+{


FreeRTOS task functions must match the TaskFunction_t prototype, which is defined as void (*)(void *). Defining a task function with no parameters (i.e., void Task_Name(void)) and passing it to xTaskCreateStatic results in a function pointer signature mismatch, which is undefined behavior in C and can lead to stack corruption or register mismatches depending on the compiler and calling convention. Please update the signature to accept a void * parameter.

Suggested change

void Task_ReadCurrents()

{

void Task_ReadCurrents(void *argument)

{

gemini-code-assist · 2026-06-25T06:55:11Z

+void Task_CanSendStatus()
+{


FreeRTOS task functions must match the TaskFunction_t prototype, which is defined as void (*)(void *). Defining a task function with no parameters (i.e., void Task_Name(void)) and passing it to xTaskCreateStatic results in a function pointer signature mismatch, which is undefined behavior in C and can lead to stack corruption or register mismatches depending on the compiler and calling convention. Please update the signature to accept a void * parameter.

void Task_CanSendStatus(void *argument) {

gemini-code-assist · 2026-06-25T06:55:11Z

+		*((uint16_t*) (ch_data+PDU_MK1_CAN_PDUSTATUS_VOLTAGE_INDEX)) = ((uint16_t) (0));     // no voltage data yet
+		*((uint16_t*) (ch_data+PDU_MK1_CAN_PDUSTATUS_CURRENT_INDEX)) = ((uint16_t) (PDU_Mk1_CurrentSensing_GetCurrentsPtr()[ch_to_send]*PDU_MK1_CAN_PDUSTATUS_CURRENT_INVFACTOR));


Casting ch_data + PDU_MK1_CAN_PDUSTATUS_CURRENT_INDEX (which resolves to an odd address, 3) to a uint16_t* and dereferencing it performs an unaligned memory access. On some ARM Cortex-M processors, this can trigger a HardFault. Furthermore, it violates strict aliasing rules in C, which can lead to undefined compiler optimizations. A safer, portable, and standard-compliant approach is to use byte shifting to serialize the 16-bit values into the byte array.

uint16_t voltage = 0; ch_data[PDU_MK1_CAN_PDUSTATUS_VOLTAGE_INDEX] = voltage & 0xFF; ch_data[PDU_MK1_CAN_PDUSTATUS_VOLTAGE_INDEX + 1] = (voltage >> 8) & 0xFF; uint16_t current = (uint16_t)(PDU_Mk1_CurrentSensing_GetCurrentsPtr()[ch_to_send] * PDU_MK1_CAN_PDUSTATUS_CURRENT_INVFACTOR); ch_data[PDU_MK1_CAN_PDUSTATUS_CURRENT_INDEX] = current & 0xFF; ch_data[PDU_MK1_CAN_PDUSTATUS_CURRENT_INDEX + 1] = (current >> 8) & 0xFF;

gemini-code-assist · 2026-06-25T06:55:11Z

+void Task_HSSControl()
+{


FreeRTOS task functions must match the TaskFunction_t prototype, which is defined as void (*)(void *). Defining a task function with no parameters (i.e., void Task_Name(void)) and passing it to xTaskCreateStatic results in a function pointer signature mismatch, which is undefined behavior in C and can lead to stack corruption or register mismatches depending on the compiler and calling convention. Please update the signature to accept a void * parameter.

Suggested change

void Task_HSSControl()

{

void Task_HSSControl(void *argument)

{

gemini-code-assist · 2026-06-25T06:55:11Z

+void Task_Init()
+{


FreeRTOS task functions must match the TaskFunction_t prototype, which is defined as void (*)(void *). Defining a task function with no parameters (i.e., void Task_Name(void)) and passing it to xTaskCreateStatic results in a function pointer signature mismatch, which is undefined behavior in C and can lead to stack corruption or register mismatches depending on the compiler and calling convention. Please update the signature to accept a void * parameter.

Suggested change

void Task_Init()

{

void Task_Init(void *argument)

{

gemini-code-assist · 2026-06-25T06:55:11Z

+		if (can_fd_send(hfdcan3, &tx_header, ch_data, portMAX_DELAY) != CAN_OK){
+			while(1)
+			{
+				printf("ERROR:CAN_SEND_PDUSTATUS\n");
+				Error_Handler();
+			}
+		}


The while(1) loop around Error_Handler() is redundant because Error_Handler() already contains an infinite loop and never returns. Simplifying this block improves code readability.

if (can_fd_send(hfdcan3, &tx_header, ch_data, portMAX_DELAY) != CAN_OK){ printf("ERROR:CAN_SEND_PDUSTATUS\n"); Error_Handler(); }

gemini-code-assist · 2026-06-25T06:55:11Z

+    task_hsscontrol = xTaskCreateStatic(Task_HSSControl,
+                    "Current Sense Task",


The task name is set to "Current Sense Task", but the task function being created is Task_HSSControl. This copy-paste error can make debugging and task monitoring confusing when using FreeRTOS tools. Please update the task name to "HSS Control Task".

task_hsscontrol = xTaskCreateStatic(Task_HSSControl, "HSS Control Task",

gemini-code-assist · 2026-06-25T06:55:11Z

+    task_hsscontrol = xTaskCreateStatic(Task_HSSControl,
+                    "Current Sense Task",


The task name is set to "Current Sense Task", but the task function being created is Task_HSSControl. This copy-paste error can make debugging and task monitoring confusing when using FreeRTOS tools. Please update the task name to "HSS Control Task".

task_hsscontrol = xTaskCreateStatic(Task_HSSControl, "HSS Control Task",

changxu-liu · 2026-06-25T06:57:08Z

+	hfdcan3->Instance = FDCAN3;
+	hfdcan3->Init.ClockDivider = FDCAN_CLOCK_DIV1;
+	hfdcan3->Init.FrameFormat = FDCAN_FRAME_CLASSIC;
+	hfdcan3->Init.Mode = FDCAN_MODE_EXTERNAL_LOOPBACK;


FDCAN_MODE_NORMAL?

changxu-liu · 2026-06-25T06:58:15Z

+
+// DEFINES --------------------------------------------------------------------
+
+#define TASK_CANSENDSTATUS_STACK_SIZE configMINIMAL_STACK_SIZE+3000


Maybe see if CAN make less f a t?

changxu-liu · 2026-06-25T06:58:38Z

+
+// DEFINES --------------------------------------------------------------------
+
+#define TASK_INIT_STACK_SIZE configMINIMAL_STACK_SIZE+5000


Also f a t?

changxu-liu · 2026-06-25T06:58:58Z

Oops remove file for now

changxu-liu · 2026-06-25T06:59:11Z

+
+// DEFINES --------------------------------------------------------------------
+
+#define TASK_READCURRENTS_STACK_SIZE configMINIMAL_STACK_SIZE+3000


changxu-liu · 2026-06-25T06:59:26Z

+    }
+}
+
+


Remove extra line.

changxu-liu · 2026-06-25T06:59:42Z

+
+        vTaskDelay(pdMS_TO_TICKS(TASK_HSSCONTROL_INTERVAL_MS));
+    }
+}


Add blank line.

changxu-liu · 2026-06-25T06:59:50Z

+    PDU_Mk1_StartTasks();
+    // task kills itself
+    vTaskDelete(NULL);
+}


Add blank line.

changxu-liu · 2026-06-25T07:03:40Z

Oops remove file for now

CAN and tasks stuff

f1138ab

changxu-liu changed the title ~~CAN and tasks stuff~~ CAN status and tasks stuff Jun 25, 2026

gemini-code-assist Bot reviewed Jun 25, 2026

View reviewed changes

changxu-liu commented Jun 25, 2026

View reviewed changes

Comment thread firmware/PowerDistributionUnit_Mk1/tasks/inc/Task_PrintStatus.h

changxu-liu Jun 25, 2026

Copy link
Copy Markdown

Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops remove file for now

changxu-liu commented Jun 25, 2026

View reviewed changes

Comment thread firmware/PowerDistributionUnit_Mk1/tasks/src/Task_Blink.c

}

}

changxu-liu Jun 25, 2026

Copy link
Copy Markdown

Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra line.

changxu-liu commented Jun 25, 2026

View reviewed changes

Comment thread firmware/PowerDistributionUnit_Mk1/tasks/src/Task_PrintStatus.c

changxu-liu Jun 25, 2026

Copy link
Copy Markdown

Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops remove file for now

changxu-liu added 2 commits June 25, 2026 00:05

remove old tests because causing Github check to fail

7ffc4ac

fix MakeAllTests github check

8e507c1

		((uint16_t) (ch_data+PDU_MK1_CAN_PDUSTATUS_VOLTAGE_INDEX)) = ((uint16_t) (0)); // no voltage data yet
		((uint16_t) (ch_data+PDU_MK1_CAN_PDUSTATUS_CURRENT_INDEX)) = ((uint16_t) (PDU_Mk1_CurrentSensing_GetCurrentsPtr()[ch_to_send]*PDU_MK1_CAN_PDUSTATUS_CURRENT_INVFACTOR));

		task_hsscontrol = xTaskCreateStatic(Task_HSSControl,
		"Current Sense Task",


		// DEFINES --------------------------------------------------------------------

		#define TASK_CANSENDSTATUS_STACK_SIZE configMINIMAL_STACK_SIZE+3000


		// DEFINES --------------------------------------------------------------------

		#define TASK_INIT_STACK_SIZE configMINIMAL_STACK_SIZE+5000


		// DEFINES --------------------------------------------------------------------

		#define TASK_READCURRENTS_STACK_SIZE configMINIMAL_STACK_SIZE+3000

Uh oh!

Conversation

changxu-liu commented Jun 25, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

changxu-liu Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

changxu-liu Jun 25, 2026 •

edited

Loading