alejoseb / Modbus-STM32-HAL-FreeRTOS

Modbus TCP and RTU, Master and Slave for STM32 using Cube HAL and FreeRTOS
GNU Lesser General Public License v2.1
579 stars 192 forks source link

Negative numbers in xTaskNotify and ulTaskNotifyTake #106

Closed AntonBiakov closed 4 months ago

AntonBiakov commented 4 months ago

Hello. Pay attention to the lines

https://github.com/alejoseb/Modbus-STM32-HAL-FreeRTOS/blob/40839603ae1f53ec2d44f1689fc24306343c7920/MODBUS-LIB/Src/Modbus.c#L476 https://github.com/alejoseb/Modbus-STM32-HAL-FreeRTOS/blob/40839603ae1f53ec2d44f1689fc24306343c7920/MODBUS-LIB/Src/Modbus.c#L1130 https://github.com/alejoseb/Modbus-STM32-HAL-FreeRTOS/blob/40839603ae1f53ec2d44f1689fc24306343c7920/MODBUS-LIB/Src/Modbus.c#L1166 https://github.com/alejoseb/Modbus-STM32-HAL-FreeRTOS/blob/40839603ae1f53ec2d44f1689fc24306343c7920/MODBUS-LIB/Src/Modbus.c#L1200

BaseType_t xTaskNotify( TaskHandle_t xTaskToNotify, uint32_t ulValue, eNotifyAction eAction );

The second parameter is a positive number, but the mb_errot_t enumeration is negative numbers.

In the code examples, a positive variable is compared to a negative number

https://github.com/alejoseb/Modbus-STM32-HAL-FreeRTOS/blob/40839603ae1f53ec2d44f1689fc24306343c7920/Examples/ModbusF429TCP/Core/Src/freertos.c#L200

alejoseb commented 4 months ago

Hi, This is not quite right, uint32_t ulValue is not a positive number as you mentioned, it is just "unsigned." Thus, this comparison is simply performed as unsigned bit vectors, and it is not ideal but safe in this particular case. The enumeration is taken from the original library, which uses negative numbers, you can update it for you personal usage if you wish.

AntonBiakov commented 4 months ago

uint32_t is truly unsigned. pass -1 to the xTaskNotify function, receive in ulTaskNotifyTake 4294967295.

I'm not forcing you to test it, but run the simple code and you'll see the result.

TaskHandle_t helloTaskHandler = NULL;

int main(void) { xTaskCreate(led_task, "LED", 256, NULL, 4, NULL); xTaskCreate(hello_task, "BOTTON", 256, NULL, 4, &helloTaskHandler); }

static void led_task(void *pvParameters) { (void) pvParameters; int note = -1; while(1) { xTaskNotify(helloTaskHandler, note, eSetValueWithOverwrite); note--; if (note < -10) { note = -1; } vTaskDelay(1000 / portTICK_PERIOD_MS); } vTaskDelete(NULL); }

static void hello_task(void *pvParameters) { (void) pvParameters; uint32_t notification; while(1) { vTaskDelay(100 / portTICK_PERIOD_MS); notification = ulTaskNotifyTake(pdTRUE, 100); PRINTF("Notify %d.\r\n", notification); } vTaskDelete(NULL); } [image: image.png]

вт, 2 июл. 2024 г. в 15:12, Alejandro Mera @.***>:

Closed #106 https://github.com/alejoseb/Modbus-STM32-HAL-FreeRTOS/issues/106 as completed.

— Reply to this email directly, view it on GitHub https://github.com/alejoseb/Modbus-STM32-HAL-FreeRTOS/issues/106#event-13361713937, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACOLF2ERLO6WHYK3ZYEQ4LZKJOFPAVCNFSM6AAAAABKG5PCGCVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJTGM3DCNZRGM4TGNY . You are receiving this because you authored the thread.Message ID: <alejoseb/Modbus-STM32-HAL-FreeRTOS/issue/106/issue_event/13361713937@ github.com>

-- С уважением Бяков Антон

alejoseb commented 4 months ago

I think you didn't understand my point and how the comparison works internally. I am aware that -1 (signed) is equivalent to 4294967295 (unsigned). The comparison is still valid since this conversion is done automatically and it is done at bit level. I am not evaluating if the number is > or <. As I said, it is not quite right to compare signed and unsigned values, since that is confusing, but In this case is valid. You can put 4294967295 instead of -1 in the enumeration for clarity, without any practical difference.