FreeRTOS / FreeRTOS-Cellular-Interface

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

Incompatible pointer types between CellularAtParseTokenHandler_t and functions in cellular_common_api.h #164

Closed thirtytwobits closed 7 months ago

thirtytwobits commented 8 months ago

Most of the implementations I've seen of this interface are like FreeRTOS-Cellular-Interface-Reference-Quectel-BG96/source/cellular_bg96_urc_handler.c where an array of CellularAtParseTokenMap_t is defined using functions like Cellular_CommonUrcProcessCereg from cellular_common_api.h:

CellularAtParseTokenMap_t CellularUrcHandlerTable[] =
{
    { "CEREG",          Cellular_CommonUrcProcessCereg },
    { "CGREG",          Cellular_CommonUrcProcessCgreg },
    { "CREG",           Cellular_CommonUrcProcessCreg  },
    { "POWERED DOWN",   _Cellular_ProcessPowerDown     },
    { "PSM POWER DOWN", _Cellular_ProcessPsmPowerDown  },
    { "QIND",           _Cellular_ProcessIndication    },
    { "QIOPEN",         _Cellular_ProcessSocketOpen    },
    { "QIURC",          _Cellular_ProcessSocketurc     },
    { "QSIMSTAT",       _Cellular_ProcessSimstat       },
    { "RDY",            _Cellular_ProcessModemRdy      }
};

If you look at CellularAtParseTokenHandler_t, however, you'll see it's defined as a function pointer to a function that does not return a value whereas Cellular_CommonUrcProcessCgreg returns a CellularError_t. Are the implementations wrong to use this function in this manner or is there a flaw in the interface design?

chinglee-iot commented 8 months ago

@thirtytwobits URC parsing error is handled in port URC handler implementation. Therefore, it is defined as a function pointer with no return value. Cellular_CommonUrcProcessCgreg is common handler implementation for 3GPP URC. The return value helps to indicate the URC parsing status to the port. Cellular port can make use of this common handler with the following example code:

void _Cellular_ProcessCgreg( CellularContext_t * pContext,
                                               char * pInputLine )
{
    CellularPktStatus_t pktStatus;
    pktStatus = Cellular_CommonUrcProcessCgreg( pContext, pInputLine );
    if( pktStatus != CELLULAR_PKT_STATUS_OK )
    {
        /* Handle different packet status error code here. */
    }
}

Thank you for creating this issue. We would like to create a PR to update BG96 port with the example above.

chinglee-iot commented 7 months ago

The BG96 implementation is updated in this PR https://github.com/FreeRTOS/FreeRTOS-Cellular-Interface-Reference-Quectel-BG96/commit/1c71555e690e24c4be3470c2734eee9a24ae07f7. Thank you for reporting this issue.