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.

Submitted by:  Douglas <ourairquality>
Submitted on:  Thu 04 Jan 2018 01:52:21 PM UTC  
 
Category:  Platform ports Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  Simon Goldschmidt <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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>

 

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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    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.5