patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9534, freertos: misc fixes, core locking...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9534: freertos: misc fixes, core locking check support.

Submitter:  Douglas <ourairquality>
Submitted:  Thu 04 Jan 2018 01:52:21 PM UTC
   
 
Category:  Platform ports Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  goldsimon Open/Closed:  Closed
Planned Release:  None

Sat 06 Jan 2018 01:25:12 PM UTC, comment #4: 

Ok, so since we have a preprocessor check that INCLUDE_vTaskDelay is enabled, we should not need the loop. Maybe there's a return value to check for in the futuer. I'd rather go the safe way and check for "OK" instead of retrying for all possible erors.

> was wondering if esp-open-rtos should follow that approach?


It helps to keep the code cleaner and keep compile times low (less headers included). You might be able to save some bytes in the generated code though when doing it your way. You'd have to check that...

Simon Goldschmidt <goldsimon>
Group administrator
Sat 06 Jan 2018 06:13:32 AM UTC, comment #3: 


> Next, why should we retry sys_mutex_lock etc. after a timeout? I don't expect such a timeout to happen when passing portMAX_DELAY, or is there something I am missing?


Looking at the source code it appears that xQueueSemaphoreTake and xQueueGenericSend can only return pdPASS when given portMAX_DELAY when INCLUDE_vTaskSuspend == 1 (which is the case for esp-open-rtos), but might some use of FreeRTOS not use INCLUDE_vTaskSuspend == 1 and actually time out in which case the caller retrying seems appropriate. In any case there does not appear to be an error return value to report.

> Moving sys_jiffies to sys_arch.h doesn't work though: FreeRTOS headers are deliberately not exported via sys_arch.h in my port.


Interesting so does that explain why there are 'struct _sys_mut' etc - was wondering if esp-open-rtos should follow that approach?

Douglas <ourairquality>
Fri 05 Jan 2018 07:55:07 PM UTC, comment #2: 

Thanks, I've merged a lot of your patches.

Moving sys_jiffies to sys_arch.h doesn't work though: FreeRTOS headers are deliberately not exported via sys_arch.h in my port.

Next, why should we retry sys_mutex_lock etc. after a timeout? I don't expect such a timeout to happen when passing portMAX_DELAY, or is there something I am missing?

I've made some things optional, as people's preferences might differ. One of them is how the stack size is interpreted.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 04 Jan 2018 02:01:27 PM UTC, comment #1: 

Observation: it might be better to make the unit of the stack size argument to sys_thread_new be stack words rather than bytes for the freertos port. FreeRTOS stack sizes are generally in units of stack words so it might be more consistent and less confusing to keep to that for lwip too. I note that TCPIP_THREAD_STACKSIZE is defined as a platform dependent value, so perhaps using a unit of stack words is ok.

Douglas <ourairquality>
Thu 04 Jan 2018 01:52:21 PM UTC, original submission:  

Make a quick pass comparing the contrib freertos example to the esp-open-rtos support for FreeRTOS and here are some suggestions to consider.

Douglas <ourairquality>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by ourairquality (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-01-06 goldsimon Open/ClosedOpen Closed
    2018-01-05 goldsimon StatusNone Done
        Assigned toNone goldsimon
    2018-01-04 ourairquality Attached File- Added 0001-freertos-misc-fixes-core-locking-check-support.patch, #42824

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code