enables UPSV=1 low power mode for R510 when idle for 9.2s

hal/inc/hal_platform.h

@@ -58,6 +58,10 @@
 # endif /* HAL_PLATFORM_CELLULAR_SERIAL */
 #endif /* HAL_PLATFORM_CELLULAR */
 
+#ifndef HAL_PLATFORM_CELLULAR_LOW_POWER
+#define HAL_PLATFORM_CELLULAR_LOW_POWER (0)
+#endif /* HAL_PLATFORM_CELLULAR_LOW_POWER */
+
 #ifndef HAL_PLATFORM_MESH_DEPRECATED
 #define HAL_PLATFORM_MESH_DEPRECATED 0
 #endif /* HAL_PLATFORM_MESH_DEPRECATED */

hal/network/lwip/ppp_client.cpp

@@ -52,6 +52,10 @@ netif_ext_callback_t Client::netifCb_ = {};
 int Client::netifClientDataIdx_ = -1;
 constexpr const char* Client::eventNames_[];
 constexpr const char* Client::stateNames_[];
+const auto NCP_CLIENT_LCP_ECHO_INTERVAL_SECONDS_DEFAULT = 5;
+const auto NCP_CLIENT_LCP_ECHO_INTERVAL_SECONDS_R510 = 240; // 4 minutes (4.25 minutes max)
+const auto NCP_CLIENT_LCP_ECHO_MAX_FAILS_DEFAULT = 10;
+const auto NCP_CLIENT_LCP_ECHO_MAX_FAILS_R510 = 1;
 
 namespace {
 
@@ -126,6 +130,13 @@ void Client::init() {
       pcb_->settings.fsm_ignore_conf_req_opened = 1;
     }
 
+    if (state_ == STATE_CONNECTED || state_ == STATE_DISCONNECTED) {
+      // Allows the R510 to drop into low power mode (USPV=1) automatically after ~9s when idle
+      pcb_->settings.lcp_echo_interval = NCP_CLIENT_LCP_ECHO_INTERVAL_SECONDS_R510;
+      pcb_->settings.lcp_echo_fails = NCP_CLIENT_LCP_ECHO_MAX_FAILS_R510;
+      pcb_->lcp_echos_pending = 0; // reset echo count
+    }
+
     pppapi_set_notify_phase_callback(pcb_, &Client::notifyPhaseCb);
 
     os_queue_create(&queue_, sizeof(QueueEvent), 5, nullptr);
@@ -235,6 +246,7 @@ int Client::input(const uint8_t* data, size_t size) {
       case STATE_DISCONNECTING:
       case STATE_CONNECTED: {
         LOG_DEBUG(TRACE, "RX: %lu", size);
+        // LOG_DUMP(TRACE, data, size);
 
         if (platform_primary_ncp_identifier() == PLATFORM_NCP_SARA_R410) {
           auto pppos = (pppos_pcb*)pcb_->link_ctx_cb;
@@ -489,6 +501,7 @@ uint32_t Client::outputCb(ppp_pcb* pcb, uint8_t* data, uint32_t len, void* ctx)
 
 uint32_t Client::output(const uint8_t* data, size_t len) {
   LOG_DEBUG(TRACE, "TX: %lu", len);
+  // LOG_DUMP(TRACE, data, len);
 
   if (oCb_) {
     auto r = oCb_(data, len, oCbCtx_);
@@ -555,6 +568,20 @@ void Client::notifyNetif(netif_nsc_reason_t reason, const netif_ext_callback_arg
 
 void Client::transition(State newState) {
   LOG(TRACE, "State %s -> %s", stateNames_[state_], stateNames_[newState]);
+  if (newState != state_) {
+    if (platform_primary_ncp_identifier() == PLATFORM_NCP_SARA_R510) {
+      if (newState == STATE_CONNECTED || newState == STATE_DISCONNECTED) {
+        // Allows the R510 to drop into low power mode (USPV=1) automatically after ~9s when idle
+        pcb_->settings.lcp_echo_interval = NCP_CLIENT_LCP_ECHO_INTERVAL_SECONDS_R510;
+        pcb_->settings.lcp_echo_fails = NCP_CLIENT_LCP_ECHO_MAX_FAILS_R510;
+        pcb_->lcp_echos_pending = 0; // reset echo count
+      } else {
+        // Resume default keep alive when not CONNECTED/DISCONNECTED for R510
+        pcb_->settings.lcp_echo_interval = NCP_CLIENT_LCP_ECHO_INTERVAL_SECONDS_DEFAULT;
+        pcb_->settings.lcp_echo_fails = NCP_CLIENT_LCP_ECHO_MAX_FAILS_DEFAULT;
+      }
+    }
+  }
   state_ = newState;
 
   {

hal/network/ncp/cellular/cellular_ncp_client.h

@@ -81,6 +81,14 @@ enum class CellularAccessTechnology {
     LTE_NB_IOT = 9
 };
 
+enum class CellularPowerSavingValue {
+    NONE = -1,
+    UPSV_DISABLED = 0,
+    UPSV_ENABLED_TIMER = 1,
+    UPSV_ENABLED_RTS = 2,
+    UPSV_ENABLED_DTR = 3,
+};
+
 enum class CellularOperationMode {
     NONE = -1,
     PS_ONLY = 0,

hal/network/ncp_client/sara/sara_ncp_client.cpp

@@ -101,7 +101,8 @@ const auto UBLOX_NCP_R4_APP_FW_VERSION_LATEST_02B_01 = 204;
 const auto UBLOX_NCP_R4_APP_FW_VERSION_0512 = 219;
 
 const auto UBLOX_NCP_MAX_MUXER_FRAME_SIZE = 1509;
-const auto UBLOX_NCP_KEEPALIVE_PERIOD = 5000; // milliseconds
+const auto UBLOX_NCP_KEEPALIVE_PERIOD_DEFAULT = 5000; // milliseconds
+const auto UBLOX_NCP_KEEPALIVE_PERIOD_R510 = 0; // disables muxer keep alive after CONNECTED
 const auto UBLOX_NCP_KEEPALIVE_MAX_MISSED = 5;
 
 // FIXME: for now using a very large buffer
@@ -199,6 +200,7 @@ int SaraNcpClient::init(const NcpClientConfig& conf) {
     firmwareInstallRespCodeR510_ = -1;
     lastFirmwareInstallRespCodeR510_ = -1;
     waitReadyRetries_ = 0;
+    sleepNoPPPWrite_ = false;
     registrationTimeout_ = REGISTRATION_TIMEOUT;
     resetRegistrationState();
     if (modemPowerState()) {
@@ -466,13 +468,21 @@ int SaraNcpClient::disconnect() {
     if (connState_ == NcpConnectionState::DISCONNECTED) {
         return SYSTEM_ERROR_NONE;
     }
+
+    SCOPE_GUARD({
+        resetRegistrationState();
+        connectionState(NcpConnectionState::DISCONNECTED);
+    });
+
+    // If we disconnect due to the AT interface being dead, a forced check
+    // is required because ready_ is true and parserError_ is 0.
+    const int r = parser_.execCommand(1000, "AT");
+    if (r != AtResponse::OK) {
+        parserError_ = r;
+    }
     CHECK(checkParser());
     CHECK_PARSER(setModuleFunctionality(CellularFunctionality::MINIMUM));
-    // CHECK_TRUE(r == AtResponse::OK, SYSTEM_ERROR_AT_NOT_OK);
 
-    resetRegistrationState();
-
-    connectionState(NcpConnectionState::DISCONNECTED);
     return SYSTEM_ERROR_NONE;
 }
 
@@ -527,7 +537,10 @@ int SaraNcpClient::dataChannelWrite(int id, const uint8_t* data, size_t size) {
         }
     }
 
-    int err = muxer_.writeChannel(UBLOX_NCP_PPP_CHANNEL, data, size);
+    int err = gsm0710::GSM0710_ERROR_NONE;
+    if (!sleepNoPPPWrite_) {
+        err = muxer_.writeChannel(UBLOX_NCP_PPP_CHANNEL, data, size);
+    }
     if (err == gsm0710::GSM0710_ERROR_FLOW_CONTROL) {
         // Not an error
         LOG_DEBUG(WARN, "Remote side flow control");
@@ -1484,6 +1497,7 @@ int SaraNcpClient::initReady(ModemState state) {
                 }
             }
         }
+
         // Check that the modem is responsive at the new baudrate
         skipAll(serial_.get(), 1000);
         CHECK(waitAtResponse(10000));
@@ -1523,12 +1537,19 @@ int SaraNcpClient::initReady(ModemState state) {
         // Disable Cat-M1 low power modes
         CHECK(disablePsmEdrx());
 
+        // Allows the R510 to drop into low power mode automatically after ~9s when idle
+        if (ncpId() == PLATFORM_NCP_SARA_R510) {
+            // XXX: R510 UPSV=1 appears to have issues dropping into low power mode unless we ensure to disable and enable on boot
+            CHECK_PARSER_OK(setPowerSavingValue(CellularPowerSavingValue::UPSV_DISABLED, true /* check */));
+            CHECK_PARSER_OK(setPowerSavingValue(CellularPowerSavingValue::UPSV_ENABLED_TIMER));
+        }
+
     } else {
         // Force Power Saving mode to be disabled
         //
         // TODO: if we enable this feature in the future add logic to CHECK_PARSER macro(s)
         // to wait longer for device to become active (see MDMParser::_atOk)
-        CHECK_PARSER_OK(parser_.execCommand("AT+UPSV=0"));
+        CHECK_PARSER_OK(setPowerSavingValue(CellularPowerSavingValue::UPSV_DISABLED, true /* check */));
     }
 
     if (state != ModemState::MuxerAtChannel) {
@@ -1702,7 +1723,11 @@ int SaraNcpClient::initMuxer() {
     // Initialize muxer
     muxer_.setStream(serial_.get());
     muxer_.setMaxFrameSize(UBLOX_NCP_MAX_MUXER_FRAME_SIZE);
-    muxer_.setKeepAlivePeriod(UBLOX_NCP_KEEPALIVE_PERIOD);
+    if (ncpId() == PLATFORM_NCP_SARA_R510 && connState_ == NcpConnectionState::DISCONNECTED) {
+        muxer_.setKeepAlivePeriod(UBLOX_NCP_KEEPALIVE_PERIOD_R510);
+    } else {
+        muxer_.setKeepAlivePeriod(UBLOX_NCP_KEEPALIVE_PERIOD_DEFAULT);
+    }
     muxer_.setKeepAliveMaxMissed(UBLOX_NCP_KEEPALIVE_MAX_MISSED);
     muxer_.setMaxRetransmissions(3);
     muxer_.setAckTimeout(UBLOX_MUXER_T1);
@@ -1780,6 +1805,35 @@ int SaraNcpClient::checkSimReadiness(bool checkForRfReset) {
     return r;
 }
 
+int SaraNcpClient::getPowerSavingValue() {
+    auto respUpsv = parser_.sendCommand("AT+UPSV?");
+    int upsvVal = -1;
+    // +UPSV: 1,2000,1
+    auto upsvValCnt = respUpsv.scanf("+UPSV: %d,%*d,%*d", &upsvVal);
+    CHECK_PARSER_OK(respUpsv.readResult());
+    CHECK_TRUE(upsvValCnt == 1, SYSTEM_ERROR_AT_RESPONSE_UNEXPECTED);
+
+    return upsvVal;
+}
+
+int SaraNcpClient::setPowerSavingValue(CellularPowerSavingValue upsv, bool check) {
+    if (check) {
+        if ((int)upsv == CHECK(getPowerSavingValue())) {
+            // Already in required state
+            return SYSTEM_ERROR_NONE;
+        }
+    }
+
+    int r = SYSTEM_ERROR_UNKNOWN;
+
+    r = parser_.execCommand(1000, "AT+UPSV=%d",(int)upsv);
+
+    CHECK_PARSER_OK(r);
+
+    // AtResponse::Result!
+    return r;
+}
+
 int SaraNcpClient::getOperationModeCached(CellularOperationMode& cemode) {
     uint32_t systemCacheOperationMode = 0;
     cemode = CellularOperationMode::NONE;
@@ -1823,7 +1877,7 @@ int SaraNcpClient::setOperationMode(CellularOperationMode cemode, bool check, bo
     }
 
     if (check) {
-        if (cemode == CHECK(getOperationMode())) {
+        if ((int)cemode == CHECK(getOperationMode())) {
             if (save) {
                 setOperationModeCached(cemode);
             }
@@ -1855,7 +1909,7 @@ int SaraNcpClient::getModuleFunctionality() {
 
 int SaraNcpClient::setModuleFunctionality(CellularFunctionality cfun, bool check) {
     if (check) {
-        if (cfun == CHECK(getModuleFunctionality())) {
+        if ((int)cfun == CHECK(getModuleFunctionality())) {
             // Already in required state
             return 0;
         }
@@ -2069,8 +2123,18 @@ int SaraNcpClient::enterDataMode() {
 
     // There is some kind of a bug in 02.19 R410 modem firmware in where it does not accept
     // any AT commands over the second muxed channel, unless we send something over channel 1 first.
-    if (ncpId() == PLATFORM_NCP_SARA_R410) {
-        CHECK(waitAtResponse(parser_, 5000));
+    //
+    // If R510 and CONNECTED already, in the case where we are in low power mode and the keepalives
+    // may be diabled/longer, just in case we are trying to resume a broken connection, let's timeout
+    // faster with an AT/OK check prior to the following AT+CGATT? call that takes 90s to timeout.
+    // This will also skip a follow up AT+CPWROFF call because ready_ will be set to false.
+    if ((ncpId() == PLATFORM_NCP_SARA_R410) ||
+            (ncpId() == PLATFORM_NCP_SARA_R510 && connState_ == NcpConnectionState::CONNECTED)) {
+        const int r = parser_.execCommand(1000, "AT");
+        if (r != AtResponse::OK) {
+            parserError_ = r;
+        }
+        CHECK(checkParser());
     }
 
     // CGATT should be enabled before we dial
@@ -2182,14 +2246,32 @@ int SaraNcpClient::getMtu() {
 int SaraNcpClient::urcs(bool enable) {
     const NcpClientLock lock(this);
     if (enable) {
+        sleepNoPPPWrite_ = false;
         CHECK_TRUE(muxer_.resumeChannel(UBLOX_NCP_AT_CHANNEL) == 0, SYSTEM_ERROR_INTERNAL);
+        if ((ncpId() != PLATFORM_NCP_SARA_R510) ||
+                ((ncpId() == PLATFORM_NCP_SARA_R510) &&
+                !(connState_ == NcpConnectionState::CONNECTED || connState_ == NcpConnectionState::DISCONNECTED))) {
+            muxer_.setKeepAlivePeriod(UBLOX_NCP_KEEPALIVE_PERIOD_DEFAULT);
+        }
         if (ncpId() == PLATFORM_NCP_SARA_U201) {
             // Make sure the modem is responsive again. U201 modems do take a while to
             // go back into functioning state
             CHECK(waitAtResponse(5000, gsm0710::proto::DEFAULT_T2));
         }
     } else {
+        sleepNoPPPWrite_ = true;
+
+        // R510 may be in low power mode, wake it up so we can get our muxer data channel suspend
+        // echo, before we get into sleep and potentially prematurely wake by "network activity" (RX data)
+        if (ncpId() == PLATFORM_NCP_SARA_R510) {
+            CHECK(waitAtResponse(5000, 2000));
+        }
         CHECK_TRUE(muxer_.suspendChannel(UBLOX_NCP_AT_CHANNEL) == 0, SYSTEM_ERROR_INTERNAL);
+
+        // muxer_.suspendChannel(UBLOX_NCP_AT_CHANNEL) does not block muxer AT channel keepalives. All modems will
+        // require this to be disabled to wake on network activity, and not prematurely wake on the echo of the keepalive.
+        muxer_.setKeepAlivePeriod(UBLOX_NCP_KEEPALIVE_PERIOD_R510);
+        HAL_Delay_Milliseconds(100); // allow a bit of time for muxer echo response
     }
     return SYSTEM_ERROR_NONE;
 }
@@ -2204,6 +2286,14 @@ void SaraNcpClient::connectionState(NcpConnectionState state) {
     LOG(TRACE, "NCP connection state changed: %d", (int)state);
     connState_ = state;
 
+    if (ncpId() == PLATFORM_NCP_SARA_R510) {
+        if (connState_ == NcpConnectionState::CONNECTED || connState_ == NcpConnectionState::DISCONNECTED) {
+            muxer_.setKeepAlivePeriod(UBLOX_NCP_KEEPALIVE_PERIOD_R510);
+        } else {
+            muxer_.setKeepAlivePeriod(UBLOX_NCP_KEEPALIVE_PERIOD_DEFAULT);
+        }
+    }
+
     if (connState_ == NcpConnectionState::CONNECTED) {
         // Reset CGATT workaround flag
         cgattWorkaroundApplied_ = false;

hal/network/ncp_client/sara/sara_ncp_client.h

@@ -121,6 +121,7 @@ class SaraNcpClient: public CellularNcpClient {
     int firmwareInstallRespCodeR510_ = 0;
     int lastFirmwareInstallRespCodeR510_ = 0;
     int waitReadyRetries_ = 0;
+    bool sleepNoPPPWrite_ = false;
 
     system_tick_t lastWindow_ = 0;
     size_t bytesInWindow_ = 0;
@@ -171,6 +172,8 @@ class SaraNcpClient: public CellularNcpClient {
     int waitAtResponseFromPowerOn(ModemState& modemState);
     int disablePsmEdrx();
     int checkSimReadiness(bool checkForRfReset = false);
+    int getPowerSavingValue();
+    int setPowerSavingValue(CellularPowerSavingValue upsv, bool check = false);
     int getOperationModeCached(CellularOperationMode& cemode);
     int setOperationModeCached(CellularOperationMode cemode);
     int getOperationMode();

hal/src/boron/hal_platform_config.h

@@ -7,6 +7,7 @@
 #define HAL_PLATFORM_NCP_AT (1)
 #define HAL_PLATFORM_CELLULAR (1)
 #define HAL_PLATFORM_CELLULAR_SERIAL (HAL_USART_SERIAL2)
+#define HAL_PLATFORM_CELLULAR_LOW_POWER (1)
 #define HAL_PLATFORM_SETUP_BUTTON_UX (1)
 #define HAL_PLATFORM_MUXER_MAY_NEED_DELAY_IN_TX (1)
 #define HAL_PLATFORM_SPI_NUM (2)

hal/src/nRF52840/sleep_hal.cpp

@@ -47,6 +47,7 @@
 #include "exrtc_hal.h"
 #endif
 #include "spark_wiring_vector.h"
+#include "platform_ncp.h"
 
 #if HAL_PLATFORM_IO_EXTENSION && MODULE_FUNCTION != MOD_FUNC_BOOTLOADER
 #if HAL_PLATFORM_MCP23S17
@@ -680,13 +681,29 @@ static bool isWokenUpByLpcomp() {
     return NVIC_GetPendingIRQ(COMP_LPCOMP_IRQn);
 }
 
-static bool isWokenUpByNetwork(const hal_wakeup_source_network_t* networkWakeup) {
+static bool isWokenUpByNetwork(const hal_wakeup_source_network_t* networkWakeup, const uint32_t ncpId) {
 // TODO: More than one network interface are supported on platform.
     if (networkWakeup->flags & HAL_SLEEP_NETWORK_FLAG_INACTIVE_STANDBY) {
         return false;
     }
 #if HAL_PLATFORM_CELLULAR
     if (networkWakeup->index == NETWORK_INTERFACE_CELLULAR && NVIC_GetPendingIRQ(UARTE1_IRQn)) {
+        // XXX: u-blox issue where RXD pin toggles to HI-Z for ~10us about 1ms after CTS goes HIGH
+        //      while modem is in UPSV=1 mode and in a low power state.  We see a low pulse due to
+        //      10k pull-down on RXD. These will occur every 1.28s.  If we wake up via R510 cellular
+        //      network activity, and CTS is HIGH, go back to sleep.
+        // Is CTS pin HIGH? (clear pending interrupt and return false)
+        if (ncpId == PLATFORM_NCP_SARA_R510) {
+            hal_pin_info_t* halPinMap = hal_pin_map();
+            uint32_t cts1Pin = NRF_GPIO_PIN_MAP(halPinMap[CTS1].gpio_port, halPinMap[CTS1].gpio_pin);
+            uint32_t cts1State = nrf_gpio_pin_read(cts1Pin);
+            if (cts1State) {
+                nrf_uarte_event_clear(NRF_UARTE1, NRF_UARTE_EVENT_RXDRDY);
+                nrf_uarte_int_enable(NRF_UARTE1, NRF_UARTE_INT_RXDRDY_MASK);
+                NVIC_ClearPendingIRQ(UARTE1_IRQn);
+                return false;
+            }
+        }
         return true;
     }
 #endif
@@ -816,6 +833,7 @@ static void fpu_sleep_prepare(void) {
 
 static int enterStopBasedSleep(const hal_sleep_config_t* config, hal_wakeup_source_base_t** wakeupReason) {
     int ret = SYSTEM_ERROR_NONE;
+    uint32_t ncpId = platform_primary_ncp_identifier(); // save before external flash is put to sleep
 
     // Detach USB
     HAL_USB_Detach();
@@ -1059,7 +1077,7 @@ static int enterStopBasedSleep(const hal_sleep_config_t* config, hal_wakeup_sour
                 break; // Stop traversing the wakeup sources list.
             } else if (wakeupSource->type == HAL_WAKEUP_SOURCE_TYPE_NETWORK) {
                 auto networkWakeup = reinterpret_cast<hal_wakeup_source_network_t*>(wakeupSource);
-                if (isWokenUpByNetwork(networkWakeup)) {
+                if (isWokenUpByNetwork(networkWakeup, ncpId)) {
                     wakeupSourceType = HAL_WAKEUP_SOURCE_TYPE_NETWORK;
                     netif = networkWakeup->index;
                     exitSleepMode = true;