check if osThreadCreate() successful

[MisterHW]

Users new to both FreeRTOS and libscpi may find that the scpi server thread does not start. The cause in my case was an insufficient default configTOTAL_HEAP_SIZE value (e.g. 15 kByte through STM32CubeMX).

Proposed treatment:

#include "assert.h"

void scpi_server_init(void) {
    osThreadId SCPIThreadId = sys_thread_new( "SCPI", scpi_server_thread, NULL,
        2 * DEFAULT_THREAD_STACKSIZE, SCPI_THREAD_PRIO );
    /* osThreadCreate() returns 0 when requested stacksize * sizeof(StackType_t) < xFreeBytesRemaining
     * see FreeRTOSConfig.h: #define configTOTAL_HEAP_SIZE ( ( size_t ) ( 25 * 1024 )
     * or increase TOTAL_HEAP_SIZE in CubeMX > FreeRTOS > Memory management settings. */
    assert(SCPIThreadId);
}

[j123b567]

TOTAL_HEAP_SIZE is for dynamic memory allocations. libscpi does not do any dynamic memory allocations in default configuration. The problem seems to be in your code - e.g. creating task with big stack which did not fit in heap.

Please keep in mind that libscpi is not thread safe and it is your responsibility to call all functions just from one thread or bad things can happen.

[martinmoene]

That's good to know.

I think the remark specifically pertains to the call to sys_thread_new() in examples/test-LwIP-netconn/scpi_server.c.

Perhaps at the minimum a note on stack/heap size may be helpful to avoid the need to track down a preventable problem.

Also checking the return value of sys_thread_new() codifies it :)

[martinmoene]

Another way may be to provide the success status, enabling the user to decide how to handle an error, like:

In examples/test-LwIP-netconn/scpi_server.h:

/* true on succes: */
bool scpi_server_init(void);
bool SCPI_AddError(int16_t err);
bool SCPI_RequestControl(void);

In examples/test-LwIP-netconn/scpi_server.c:

bool SCPI_RequestControl(void) {
    ...
    return pdPASS == xQueueSend(user_data.evtQueue, &msg, 1000);
}

bool SCPI_AddError(int16_t err) {
    ...
    return pdPASS == xQueueSend(user_data.evtQueue, &msg, 1000);
}

bool StartDefaultTask(void const * argument) {
    return (sys_thread_t) 0 != sys_thread_new("SCPI", scpi_server_thread, NULL, 2 * DEFAULT_THREAD_STACKSIZE, SCPI_THREAD_PRIO);
}

I realize it may seem or be a bit much of a comment on what is 'just' an example. However, the propagation of the success state may help to detect the failure to create the scpi_server_thread, which appeared to be not so unlikely.

Not to forget, thank you for the very useful library!

[j123b567]

sys_thread_new

According to LwIP documentation, sys_thread_new should never fail. If it fails, it should be handled by assertion in the sys_thread_new itself.

ATTENTION: although this function returns a value, it MUST NOT FAIL (ports have to assert this!)

If it can fail, you have broken implementation of that function in your code.

xQueueCreate and others

Result of xQueueCreate and createServer of that example should be checkd. But simple assert or LWIP_ASSERT should be sufficient. It does not make sense to continue, if it fails.

xQueueSend

Result of xQueueSend depends on your needs. This is just an example and you should probably modify, extend or just take inspiration from that code. It is not intended to be used as is. You would like to modify the timeout value for xQueueSend, because the example value does not make sense for correct handling of errors and it can be also too long for many cases.

new/fixed examples

If anybody of you has implementation of any communication layer (extend/fix current examples, provide new examples, ...) and if it is possible to distill the core functionality and publish it with (commercially) usable license (BSD, MIT, Apache 2). Feel free to make a pull request.

I must put the note to the readme file, that examples are not intended for production use and they are mainly contributed by others to share some knowledge.

[MisterHW]

Thank you for looking into what started out as a luxury problem with example code and not the library itself. I recognize that in that scope it's a side quest and I should probably adjust the issue title to be more concise.

I'll mention a few STM32-related aspects below as probably the lack of assertions is STM32Cube-specific.

If [lwip/system/os/sys_arch.c: sys_thread_new()] can fail, you have broken implementation of that function in your code.

STM32Cube middleware packages are still a minefield (e.g. for STM32 users this article is indispensable before they even get to use libscpi: https://community.st.com/s/question/0D50X0000BOtfhnSQB/how-to-make-ethernet-and-lwip-working-on-stm32 ) so we can fully agree on its state. There is no assert() or LWIP_ASSERT in the sys_thread_new() generated:

sys_thread_t sys_thread_new(...)
...
return osThreadCreate(&os_thread_def, arg);

Furthermore, adding LwIP middleware in CubeMX produces the code below which again does not assert success:

/* USER CODE BEGIN OS_THREAD_DEF_CREATE_CMSIS_RTOS_V1 */
  osThreadDef(LinkThr, ethernetif_set_link, osPriorityBelowNormal, 0, configMINIMAL_STACK_SIZE * 2);
  osThreadCreate (osThread(LinkThr), &link_arg);
/* USER CODE END OS_THREAD_DEF_CREATE_CMSIS_RTOS_V1 */

re: new/fixed examples

I am currently working on what one might call an extended example for libscpi on STM32 Nucleo-F7xx boards (with FreeRTOS and LwIP middlewares) and intend to write up on the major pitfalls. I will link to it once documentation is available.

[j123b567]

STM32Cube middleware packages are still a minefield

STM32Cube is still blooming meadow in comparison to Microchip Harmony. :rofl:

You can use osThreadCreate directly and don't use sys_thread_new at all. It will clarify much.

My example is also inconsistant. It uses sys_thread_new to be OS agnostic but later use xQueue... to be FreeRTOS specific.

Rewriting it to be CMSIS-RTOS/CMSIS-RTOS2 API makes more sense.

[j123b567]

I'm not enough familiar with CMSIS-RTOS API so I fixed the example just by rewriting the problematic sys_thread_new with FreeRTOS API directly and adding some asserts, so it will at least crash with a sensible message.

[martinmoene]

Thank you very much :)