FreeRTOS / FreeRTOS-Cellular-Interface

FreeRTOS Cellular Interface implementation of the 3GPP TS v27.007 standard.
MIT License
85 stars 59 forks source link

Typo in CellularContext data structure #124

Closed tpecar-ltek closed 1 year ago

tpecar-ltek commented 1 year ago

Minor nuisance, but still

https://github.com/FreeRTOS/FreeRTOS-Cellular-Interface/blob/b0e43466642136b9bf9e64b756761220f96cf6d8/source/include/private/cellular_common_internal.h#L153-L154

The annoying thing about it is that correcting it might break modem implementations.

In a C11 context (and not compaining in earlier versions unless -pedantic is used) it might be possible to make the transition via anonymous union

struct CellularContext
{
    /* ... */

    /* Module Context. */
    union {
        void * pModueContext __attribute__((deprecated));
        void * pModuleContext;
    };
};

However, the above example should be reworked if MSVC support is required.

rawalexe commented 1 year ago

Thank you for reaching out, we are looking into it and will get back to you soon.

chinglee-iot commented 1 year ago

Hi tpecar, Thank you for your feedback. This is something we can improve. pModueContext is an internal variable to cellular common. Modem porting should use cellular common API "_Cellular_GetModuleContext" to acquire the module context. So it can be fixed directly without breaking modem implementation. We will fix this problem soon.

tpecar-ltek commented 1 year ago

Thank you for the clarification. I would like to point out that this anti-pattern is used within the sim70x0 port https://github.com/FreeRTOS/FreeRTOS-Cellular-Interface-Community-Supported-Ports/search?q=pModueContext

which is why I initially thought it would be a problem. Will open an issue there.

chinglee-iot commented 1 year ago

We would also update the sim70x0 port so that it works with both version of cellular interface.

chinglee-iot commented 1 year ago

The typo is fixed in this commit. The sim70x0 port is also updated. Thank you for your feedback.