From 11b6049c1afb75422560aef79200b39b9b439389 Mon Sep 17 00:00:00 2001 From: RobMeades Date: Wed, 25 Sep 2024 14:01:54 +0100 Subject: [PATCH] Revert previous change and fix the actual problem, which is that we should not bring down CMUX when we bring PPP down as the registration status callback that is meant to follow the reconnection is queued on the "CMUX" AT Client's callback queue, which will of course be destroyed when CMUX is brought down, D'oh. --- cell/src/u_cell_net.c | 27 +++++++++++++-------------- cell/src/u_cell_ppp.c | 29 ++++++++++++++++++++++++++++- cell/src/u_cell_ppp_shared.h | 19 +++++++++++++++++++ 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/cell/src/u_cell_net.c b/cell/src/u_cell_net.c index 1661a4aa..6a2efed1 100644 --- a/cell/src/u_cell_net.c +++ b/cell/src/u_cell_net.c @@ -246,6 +246,7 @@ static void registrationStatusCallback(uAtClientHandle_t atHandle, (void) atHandle; + if (pStatus != NULL) { if (pStatus->pCallback != NULL) { pStatus->pCallback(pStatus->domain, pStatus->networkStatus, @@ -303,7 +304,13 @@ static void activateContextCallback(uAtClientHandle_t atHandle, (uSockStringToAddress(buffer, &address) == 0)) { pIpAddress = &address.ipAddress; } + // Reconnect PPP, making sure not to disable CMUX while doing + // so since it is pointless taking it down and up again when + // that would also result in the removal of the AT client + // that it is actually using at the time + uCellPppSetDoNotDisableCmuxOnClose(cellHandle, true); uPortPppReconnect(cellHandle, pIpAddress); + uCellPppSetDoNotDisableCmuxOnClose(cellHandle, false); } } @@ -318,7 +325,6 @@ static void setNetworkStatus(uCellPrivateInstance_t *pInstance, uCellNetRegistationStatus_t *pStatus; bool printAllowed = true; uCellNetRat_t previousRat = pInstance->rat[regType]; - bool forceRegistrationStatusCallback = false; #if U_CFG_OS_CLIB_LEAKS // If we're in a URC and the C library leaks memory // when printf() is called from a dynamically @@ -452,12 +458,6 @@ static void setNetworkStatus(uCellPrivateInstance_t *pInstance, activateContextCallback, pInstance); } pInstance->profileState = U_CELL_PRIVATE_PROFILE_STATE_SHOULD_BE_UP; - // After having successfully re-registered we can end up - // falling into the logic below concerning the order of - // +CxREG URCs, when in fact we really _must_ call the - // users registration callback in this case, they need to - // know. Hence we force it to be caleld. - forceRegistrationStatusCallback = true; } } @@ -485,13 +485,12 @@ static void setNetworkStatus(uCellPrivateInstance_t *pInstance, // the user we're not registered when we are, so don't call the // callback for a "not registered" +CGREG/+CEREG if there is still // a "registered" +CGREG/+CEREG. - if (!forceRegistrationStatusCallback && - (((regType == U_CELL_PRIVATE_NET_REG_TYPE_CGREG) && - !U_CELL_NET_STATUS_MEANS_REGISTERED(pInstance->networkStatus[U_CELL_PRIVATE_NET_REG_TYPE_CGREG]) && - U_CELL_NET_STATUS_MEANS_REGISTERED(pInstance->networkStatus[U_CELL_PRIVATE_NET_REG_TYPE_CEREG])) || - ((regType == U_CELL_PRIVATE_NET_REG_TYPE_CEREG) && - !U_CELL_NET_STATUS_MEANS_REGISTERED(pInstance->networkStatus[U_CELL_PRIVATE_NET_REG_TYPE_CEREG]) && - U_CELL_NET_STATUS_MEANS_REGISTERED(pInstance->networkStatus[U_CELL_PRIVATE_NET_REG_TYPE_CGREG])))) { + if (((regType == U_CELL_PRIVATE_NET_REG_TYPE_CGREG) && + !U_CELL_NET_STATUS_MEANS_REGISTERED(pInstance->networkStatus[U_CELL_PRIVATE_NET_REG_TYPE_CGREG]) && + U_CELL_NET_STATUS_MEANS_REGISTERED(pInstance->networkStatus[U_CELL_PRIVATE_NET_REG_TYPE_CEREG])) || + ((regType == U_CELL_PRIVATE_NET_REG_TYPE_CEREG) && + !U_CELL_NET_STATUS_MEANS_REGISTERED(pInstance->networkStatus[U_CELL_PRIVATE_NET_REG_TYPE_CEREG]) && + U_CELL_NET_STATUS_MEANS_REGISTERED(pInstance->networkStatus[U_CELL_PRIVATE_NET_REG_TYPE_CGREG]))) { // We remain registered on a PS domain, nothing more to do } else { pStatus = (uCellNetRegistationStatus_t *) pUPortMalloc(sizeof(*pStatus)); diff --git a/cell/src/u_cell_ppp.c b/cell/src/u_cell_ppp.c index c2cc0e77..9913fc70 100644 --- a/cell/src/u_cell_ppp.c +++ b/cell/src/u_cell_ppp.c @@ -150,6 +150,7 @@ typedef struct { bool receiveBufferIsMalloced; bool muxAlreadyEnabled; bool uartSleepWakeOnDataWasEnabled; + bool doNotDisableCmux; } uCellPppContext_t; /* ---------------------------------------------------------------- @@ -347,7 +348,7 @@ static void closePpp(uCellPrivateInstance_t *pInstance, pContext->pDeviceSerial = NULL; } } - if (!pContext->muxAlreadyEnabled) { + if (!pContext->muxAlreadyEnabled && !pContext->doNotDisableCmux) { // Disable the multiplexer if one was in use // and it was us who started it uCellMuxPrivateDisable(pInstance); @@ -608,6 +609,32 @@ int32_t uCellPppClose(uDeviceHandle_t cellHandle, return errorCode; } +// Set whether CMUX is disabled on PPP closure or not. +void uCellPppSetDoNotDisableCmuxOnClose(uDeviceHandle_t cellHandle, + bool doNotDisableCmux) +{ + uCellPrivateInstance_t *pInstance; + uCellPppContext_t *pContext; + + if (gUCellPrivateMutex != NULL) { + + U_PORT_MUTEX_LOCK(gUCellPrivateMutex); + + pInstance = pUCellPrivateGetInstance(cellHandle); + if (pInstance != NULL) { + if (U_CELL_PRIVATE_HAS(pInstance->pModule, + U_CELL_PRIVATE_FEATURE_PPP)) { + pContext = (uCellPppContext_t *) pInstance->pPppContext; + if (pContext != NULL) { + pContext->doNotDisableCmux = doNotDisableCmux; + } + } + } + + U_PORT_MUTEX_UNLOCK(gUCellPrivateMutex); + } +} + // Transmit a buffer of data over the PPP interface. int32_t uCellPppTransmit(uDeviceHandle_t cellHandle, const char *pData, size_t dataSize) diff --git a/cell/src/u_cell_ppp_shared.h b/cell/src/u_cell_ppp_shared.h index 359bbdfa..386be461 100644 --- a/cell/src/u_cell_ppp_shared.h +++ b/cell/src/u_cell_ppp_shared.h @@ -158,6 +158,25 @@ bool uCellPppIsOpen(uDeviceHandle_t cellHandle); int32_t uCellPppClose(uDeviceHandle_t cellHandle, bool pppTerminateRequired); +/** Normally, closure of PPP would also cause CMUX to be + * deactivated if this code knows that CMUX was only + * activated in order to run PPP. This is unnecesary if the + * closure of PPP is only temporary, e.g. to recover from a + * service outage, and it is going to be brought back up again + * immediately. Call this function with doNotDisableCmux set + * to true before calling uCellPppClose() where this is the + * case (though don't forget to call it again with + * doNotDisableCmux set to false before you call uCellPppClose() + * with no intention of calling uCellPppOpen() again afterwards). + * + * @param doNotDisableCmux if true then uCellPppClose() will + * leave CMUX up, else it will close + * CMUX if CMUX was only brought up + * to run PPP. + */ +void uCellPppSetDoNotDisableCmuxOnClose(uDeviceHandle_t cellHandle, + bool doNotDisableCmux); + /** Transmit data over the PPP interface of the cellular module. * This may be integrated into a higher layer, e.g. the PPP * interface at the bottom of an IP stack, to permit it to send