Skip to content

Commit

Permalink
Revert previous change and fix the actual problem, which is that we s…
Browse files Browse the repository at this point in the history
…hould 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.
  • Loading branch information
RobMeades committed Sep 25, 2024
1 parent 49390b2 commit 11b6049
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 15 deletions.
27 changes: 13 additions & 14 deletions cell/src/u_cell_net.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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));
Expand Down
29 changes: 28 additions & 1 deletion cell/src/u_cell_ppp.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ typedef struct {
bool receiveBufferIsMalloced;
bool muxAlreadyEnabled;
bool uartSleepWakeOnDataWasEnabled;
bool doNotDisableCmux;
} uCellPppContext_t;

/* ----------------------------------------------------------------
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
19 changes: 19 additions & 0 deletions cell/src/u_cell_ppp_shared.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 11b6049

Please sign in to comment.