From 965f6f0e2621808fb069458291194073737e86c8 Mon Sep 17 00:00:00 2001 From: keeramis Date: Tue, 6 Oct 2020 21:23:50 -0500 Subject: [PATCH 1/6] Remove rssi/qual --- hal/inc/cellular_hal.h | 2 +- hal/inc/cellular_hal_constants.h | 6 -- hal/inc/hal_dynalib_cellular.h | 2 +- hal/network/ncp/cellular/cellular_hal.cpp | 53 +--------------- hal/shared/cellular_enums_hal.h | 5 +- hal/src/electron/cellular_hal.cpp | 6 +- hal/src/electron/cellular_internal.cpp | 7 +-- hal/src/electron/cellular_internal.h | 2 +- hal/src/electron/modem/mdm_hal.cpp | 27 -------- test/unit_tests/cellular/cellular.cpp | 61 +++++++------------ user/tests/wiring/no_fixture/cellular.cpp | 15 ----- wiring/inc/spark_wiring_cellular_printable.h | 7 --- wiring/src/spark_wiring_cellular.cpp | 14 +---- .../src/spark_wiring_cellular_printable.cpp | 12 +--- 14 files changed, 36 insertions(+), 183 deletions(-) diff --git a/hal/inc/cellular_hal.h b/hal/inc/cellular_hal.h index 4263b371db..282627c718 100644 --- a/hal/inc/cellular_hal.h +++ b/hal/inc/cellular_hal.h @@ -159,7 +159,7 @@ void cellular_cancel(bool cancel, bool calledFromISR, void* reserved); /** * Retrieve cellular signal strength info */ -cellular_result_t cellular_signal(CellularSignalHal* signal, cellular_signal_t* reserved); +cellular_result_t cellular_signal(cellular_signal_t* reserved); /** * Send an AT command and wait for response, optionally specify a callback function to parse the results diff --git a/hal/inc/cellular_hal_constants.h b/hal/inc/cellular_hal_constants.h index ead32617ab..4a27566332 100644 --- a/hal/inc/cellular_hal_constants.h +++ b/hal/inc/cellular_hal_constants.h @@ -74,12 +74,6 @@ struct CellularDevice typedef struct CellularDevice CellularDevice; #endif -typedef struct -{ - int rssi; - int qual; -} CellularSignalHal; - typedef struct { uint16_t size; diff --git a/hal/inc/hal_dynalib_cellular.h b/hal/inc/hal_dynalib_cellular.h index 4666d0eca6..8f9b2c5da5 100644 --- a/hal/inc/hal_dynalib_cellular.h +++ b/hal/inc/hal_dynalib_cellular.h @@ -54,7 +54,7 @@ DYNALIB_FN(13, hal_cellular, cellular_cancel, void(bool, bool, void*)) DYNALIB_FN(14, hal_cellular, HAL_NET_SetNetWatchDog, uint32_t(uint32_t)) DYNALIB_FN(15, hal_cellular, inet_gethostbyname, int(const char*, uint16_t, HAL_IPAddress*, network_interface_t, void*)) DYNALIB_FN(16, hal_cellular, inet_ping, int(const HAL_IPAddress*, network_interface_t, uint8_t, void*)) -DYNALIB_FN(17, hal_cellular, cellular_signal, cellular_result_t(CellularSignalHal*, cellular_signal_t*)) +DYNALIB_FN(17, hal_cellular, cellular_signal, cellular_result_t(cellular_signal_t*)) DYNALIB_FN(18, hal_cellular, cellular_command, cellular_result_t(_CALLBACKPTR_MDM, void*, system_tick_t, const char*, ...)) DYNALIB_FN(19, hal_cellular, cellular_data_usage_set, cellular_result_t(CellularDataHal*,void*)) DYNALIB_FN(20, hal_cellular, cellular_data_usage_get, cellular_result_t(CellularDataHal*,void*)) diff --git a/hal/network/ncp/cellular/cellular_hal.cpp b/hal/network/ncp/cellular/cellular_hal.cpp index 12c2819f34..0b8babe13b 100644 --- a/hal/network/ncp/cellular/cellular_hal.cpp +++ b/hal/network/ncp/cellular/cellular_hal.cpp @@ -294,7 +294,7 @@ bool cellular_sim_ready(void* reserved) { void cellular_cancel(bool cancel, bool calledFromISR, void* reserved) { } -int cellular_signal(CellularSignalHal* signal, cellular_signal_t* signalExt) { +int cellular_signal(cellular_signal_t* signalExt) { const auto mgr = cellularNetworkManager(); CHECK_TRUE(mgr, SYSTEM_ERROR_UNKNOWN); const auto client = mgr->ncpClient(); @@ -303,56 +303,7 @@ int cellular_signal(CellularSignalHal* signal, cellular_signal_t* signalExt) { CHECK(client->getSignalQuality(&s)); const auto strn = s.strength(); const auto qual = s.quality(); - if (signal) { - // Compatibility with Gen 2 - if (strn != 99 && strn != 255) { - int compatStrn = strn; - switch (s.strengthUnits()) { - case CellularStrengthUnits::RXLEV: { - // Leave as-is - break; - } - case CellularStrengthUnits::RSCP: { - // Simply re-map from [0-96] to [0-63] - compatStrn = (compatStrn * 63) / 96; - break; - } - case CellularStrengthUnits::RSRP: { - // Simply remap from [0-97] to [0-63] - compatStrn = (compatStrn * 63) / 97; - break; - } - } - // -113 to -50dBm - signal->rssi = -113 + compatStrn; - } - // see 3GPP TS 45.008 [20] subclause 8.2.4 - static const char compatQualMap[] = { 49, 43, 37, 25, 19, 13, 7, 0 }; - if (qual != 99 && qual != 255) { - int compatQual = qual; - switch (s.qualityUnits()) { - case CellularQualityUnits::RXQUAL: - case CellularQualityUnits::MEAN_BEP: { - // Leave as-is - break; - } - case CellularQualityUnits::ECN0: { - // Re-map from [0-49] to [0-7]. Table in UBX-13002752 - R62 (7.2.4) - compatQual = 7 - ((std::max(std::min(compatQual, 44), 2) - 2) / 6); - break; - } - case CellularQualityUnits::RSRQ: { - // Re-map from [0-34] to [0-7]. Table in UBX-13002752 - R62 (7.2.4) - compatQual = (compatQual < 10) ? (compatQual / 5) : ((std::min(compatQual, 30) - 10) / 4) + 2; - break; - } - } - // Just in case validate that we are not going to go out of bounds - if (compatQual >= 0 && compatQual <= 7) { - signal->qual = compatQualMap[compatQual]; - } - } - } + if (signalExt) { signalExt->rat = fromCellularAccessTechnology(s.accessTechnology()); // Signal strength diff --git a/hal/shared/cellular_enums_hal.h b/hal/shared/cellular_enums_hal.h index 0e91371e8f..2447af5c6f 100644 --- a/hal/shared/cellular_enums_hal.h +++ b/hal/shared/cellular_enums_hal.h @@ -190,10 +190,7 @@ typedef struct { Reg psd; //!< PSD Registration status (Packet Switched Data) Reg eps; //!< EPS registration status (Evolved Packet System) AcT act; //!< Access Technology - int rssi; //!< Received Signal Strength Indication (in dBm, range -113..-53) - int qual; //!< In UMTS RAT indicates the Energy per Chip/Noise ratio in dB levels - //!< of the current cell (see in +CGED command description), - //!< see 3GPP TS 45.008 [20] subclause 8.2.4 + union { int rxlev; //!< GSM RAT: RXLEV ([0, 63], 99), see 3GPP TS 45.008 subclause 8.1.4 int rscp; //!< UMTS RAT: RSCP ([-5, 91], 255), see 3GPP TS 25.133 subclause 9.1.1.3 diff --git a/hal/src/electron/cellular_hal.cpp b/hal/src/electron/cellular_hal.cpp index 1adfac8953..aff57c2d72 100644 --- a/hal/src/electron/cellular_hal.cpp +++ b/hal/src/electron/cellular_hal.cpp @@ -227,16 +227,16 @@ void cellular_cancel(bool cancel, bool calledFromISR, void*) } } -cellular_result_t cellular_signal(CellularSignalHal* signal, cellular_signal_t* signalext) +cellular_result_t cellular_signal(cellular_signal_t* signalext) { - if (signal == nullptr && signalext == nullptr) { + if (signalext == nullptr) { return SYSTEM_ERROR_INVALID_ARGUMENT; } NetStatus status; bool r = electronMDM.getSignalStrength(status); - return particle::detail::cellular_signal_impl(signal, signalext, r, status); + return particle::detail::cellular_signal_impl(signalext, r, status); } cellular_result_t cellular_command(_CALLBACKPTR_MDM cb, void* param, diff --git a/hal/src/electron/cellular_internal.cpp b/hal/src/electron/cellular_internal.cpp index fa391d9ff5..f0dcfd0eb5 100644 --- a/hal/src/electron/cellular_internal.cpp +++ b/hal/src/electron/cellular_internal.cpp @@ -12,7 +12,7 @@ namespace particle { namespace detail { -cellular_result_t cellular_signal_impl(CellularSignalHal* signal, cellular_signal_t* signalext, bool strengthResult, const NetStatus& status) { +cellular_result_t cellular_signal_impl(cellular_signal_t* signalext, bool strengthResult, const NetStatus& status) { // % * 100, see 3GPP TS 45.008 8.2.4 // 0.14%, 0.28%, 0.57%, 1.13%, 2.26%, 4.53%, 9.05%, 18.10% static const uint16_t berMapping[] = {14, 28, 57, 113, 226, 453, 905, 1810}; @@ -23,11 +23,6 @@ cellular_result_t cellular_signal_impl(CellularSignalHal* signal, cellular_signa return SYSTEM_ERROR_UNKNOWN; } - if (signal != nullptr) { - signal->rssi = status.rssi; - signal->qual = status.qual; - } - if (signalext != nullptr) { switch (status.act) { case ACT_GSM: diff --git a/hal/src/electron/cellular_internal.h b/hal/src/electron/cellular_internal.h index c1dc173f55..987ea1bb60 100644 --- a/hal/src/electron/cellular_internal.h +++ b/hal/src/electron/cellular_internal.h @@ -40,7 +40,7 @@ cellular_result_t cellular_data_usage_get_impl(CellularDataHal &data, const MDM_ /* detail functions defined for unit tests */ namespace particle { namespace detail { -cellular_result_t cellular_signal_impl(CellularSignalHal* signal, cellular_signal_t* signalext, bool strengthResult, const NetStatus& status); +cellular_result_t cellular_signal_impl(cellular_signal_t* signalext, bool strengthResult, const NetStatus& status); }} // namespace particle detail diff --git a/hal/src/electron/modem/mdm_hal.cpp b/hal/src/electron/modem/mdm_hal.cpp index 897464ef40..c4f082735c 100644 --- a/hal/src/electron/modem/mdm_hal.cpp +++ b/hal/src/electron/modem/mdm_hal.cpp @@ -2160,15 +2160,6 @@ int MDMParser::_cbUCGED(int type, const char* buf, int len, NetStatus* status) status->rsrp = 255; } - // Compatibility values for old API - if (status->rsrp != 255) { - // Simply remap from [0-97] to [0-63] - int compatStrn = (status->rsrp * 63) / 97; - // -113 to -50dBm - status->rssi = -113 + compatStrn; - } else { - status->rssi = 0; - } } // RSRQ maps from dBm [-20,-3] to [0,34] // We are defining hard boundaries for RSRP to be between [], @@ -2185,17 +2176,6 @@ int MDMParser::_cbUCGED(int type, const char* buf, int len, NetStatus* status) status->rsrq = 255; } - // Compatibility values for old API - if (status->rsrq != 255) { - // Re-map from [0-34] to [0-7]. Table in UBX-13002752 - R62 (7.2.4) - int compatQual = (status->rsrq < 10) ? (status->rsrq / 5) : ((std::min(status->rsrq, 30) - 10) / 4) + 2; - // Just in case validate that we are not going to go out of bounds - if (compatQual >= 0 && compatQual <= 7) { - status->qual = compatQualMap[compatQual]; - } - } else { - status->qual = 0; - } } } return WAIT; @@ -2207,9 +2187,6 @@ int MDMParser::_cbCSQ(int type, const char* buf, int len, NetStatus* status) int a,b; // +CSQ: , if (sscanf(buf, "\r\n+CSQ: %d,%d",&a,&b) == 2) { - if (a != 99) status->rssi = -113 + 2*a; // 0: -113 1: -111 ... 30: -53 dBm with 2 dBm steps - if ((b != 99) && (b < (int)sizeof(compatQualMap))) status->qual = compatQualMap[b]; // - switch (status->act) { case ACT_GSM: case ACT_EDGE: @@ -3571,10 +3548,6 @@ void MDMParser::dumpNetStatus(NetStatus *status) const char* txtAct[] = { "Unknown", "GSM", "Edge", "3G", "CDMA" }; if (status->act < sizeof(txtAct)/sizeof(*txtAct) && (status->act != ACT_UNKNOWN)) MDM_PRINTF(" Access Technology: %s\r\n", txtAct[status->act]); - if (status->rssi) - MDM_PRINTF(" Signal Strength: %d dBm\r\n", status->rssi); - if (status->qual) - MDM_PRINTF(" Signal Quality: %d\r\n", status->qual); if (status->cgi.mobile_country_code != 0) MDM_PRINTF(" Mobile Country Code: %d\r\n", status->cgi.mobile_country_code); if (status->cgi.mobile_network_code != 0) diff --git a/test/unit_tests/cellular/cellular.cpp b/test/unit_tests/cellular/cellular.cpp index 21effd8592..8d881cab98 100644 --- a/test/unit_tests/cellular/cellular.cpp +++ b/test/unit_tests/cellular/cellular.cpp @@ -147,7 +147,7 @@ TEST_CASE("cellular_signal()") { SECTION("failure to query the modem") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, false, NetStatus()) < SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, false, NetStatus()) < SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_NONE); SECTION("CellularSignal") { @@ -161,19 +161,6 @@ TEST_CASE("cellular_signal()") { } } - SECTION("CellularSignalHal RSSI and QUAL") { - NetStatus status = {}; - status.rssi = -50; // arbitrary non-zero values - status.qual = 37; // " - - SECTION("CellularSignalHal::rssi and CellularSignalHal::qual are set to expected values") { - CellularSignalHal signal = {}; - REQUIRE(cellular_signal_impl(&signal, nullptr, true, status) == SYSTEM_ERROR_NONE); - REQUIRE(signal.rssi == -50); - REQUIRE(signal.qual == 37); - } - } - SECTION("UNKNOWN") { NetStatus status = {}; status.act = ACT_UNKNOWN; @@ -188,12 +175,10 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_UNKNOWN); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_UNKNOWN); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_NONE); REQUIRE(sig.strength == 0); REQUIRE(sig.quality == 0); - REQUIRE(sig.rssi == std::numeric_limits::min()); - REQUIRE(sig.qual == std::numeric_limits::min()); SECTION("CellularSignal") { CellularSignal cs; @@ -226,7 +211,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_GSM); REQUIRE(sig.strength < 0); REQUIRE(sig.quality < 0); @@ -250,7 +235,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_GSM); REQUIRE(sig.strength >= 0); REQUIRE(sig.quality < 0); @@ -274,7 +259,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_GSM); REQUIRE(sig.strength == 0); // Inverted @@ -299,7 +284,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_GSM); REQUIRE(std::abs(sig.strength - 32767) <= 655); REQUIRE(std::abs(sig.quality - 32767) <= 6553); @@ -323,7 +308,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_GSM); REQUIRE(sig.strength == 65535); // Inverted @@ -362,7 +347,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_EDGE); REQUIRE(sig.strength < 0); REQUIRE(sig.quality < 0); @@ -386,7 +371,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_EDGE); REQUIRE(sig.strength >= 0); REQUIRE(sig.quality < 0); @@ -410,7 +395,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_EDGE); REQUIRE(sig.strength == 0); // Inverted @@ -435,7 +420,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_EDGE); REQUIRE(std::abs(sig.strength - 32767) <= 655); REQUIRE(std::abs(sig.quality - 32767) <= 6553); @@ -459,7 +444,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_EDGE); REQUIRE(sig.strength == 65535); // Inverted @@ -489,7 +474,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_UTRAN); REQUIRE(sig.strength < 0); REQUIRE(sig.quality < 0); @@ -513,7 +498,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_UTRAN); REQUIRE(sig.strength == 0); REQUIRE(sig.quality == 0); @@ -537,7 +522,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_UTRAN); REQUIRE(sig.strength == 32767); REQUIRE(std::abs(sig.quality - 32767) <= 655 * 2); @@ -561,7 +546,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_UTRAN); REQUIRE(sig.strength == 65535); REQUIRE(sig.quality == 65535); @@ -598,7 +583,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == data.expected_act); REQUIRE(sig.strength < 0); REQUIRE(sig.quality < 0); @@ -622,7 +607,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == data.expected_act); REQUIRE(sig.strength == 0); REQUIRE(sig.quality == 0); @@ -646,7 +631,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == data.expected_act); REQUIRE(sig.strength == 32429); REQUIRE(std::abs(sig.quality - 32767) <= 655 * 2); @@ -670,7 +655,7 @@ TEST_CASE("cellular_signal()") { cellular_signal_t sig = {}; sig.size = sizeof(sig); - REQUIRE(cellular_signal_impl(nullptr, &sig, true, status) == SYSTEM_ERROR_NONE); + REQUIRE(cellular_signal_impl(&sig, true, status) == SYSTEM_ERROR_NONE); REQUIRE(sig.rat == data.expected_act); REQUIRE(sig.strength == 65535); REQUIRE(sig.quality == 65535); @@ -696,12 +681,12 @@ TEST_CASE("cellular_printable") { SECTION("CellularSignal::printTo") { CellularSignal cs; - cs.rssi = -50; - cs.qual = 37; + cs.getStrengthValue() = -83.70; + cs.getQualityValue = -7.00; ser.print(cs); // printf("%s", output); - REQUIRE(strncmp(output, "-50,37", sizeof(output)) == 0); + REQUIRE(strncmp(output, "-80.00,-7.00", sizeof(output)) == 0); } SECTION("CellularData::printTo") { diff --git a/user/tests/wiring/no_fixture/cellular.cpp b/user/tests/wiring/no_fixture/cellular.cpp index f18b908544..2e2c24a5fd 100644 --- a/user/tests/wiring/no_fixture/cellular.cpp +++ b/user/tests/wiring/no_fixture/cellular.cpp @@ -93,21 +93,6 @@ test(CELLULAR_03_resolve) { assertEqual(addr, 0); } -test(CELLULAR_04_rssi_is_valid) { - connect_to_cloud(6*60*1000); - CellularSignal s; - for (int x = 0; x < 10; x++) { - s = Cellular.RSSI(); - if (s.rssi < 0) { - break; - } - Serial.println(s); - delay(5000); - } - assertLessOrEqual(s.rssi, -20); - assertMoreOrEqual(s.rssi, -150); -} - // Collects cellular signal strength and quality and validates Accesstechnology (RAT) test(CELLULAR_05_sigstr_is_valid) { connect_to_cloud(6*60*1000); diff --git a/wiring/inc/spark_wiring_cellular_printable.h b/wiring/inc/spark_wiring_cellular_printable.h index 443891dc57..73a49465f1 100644 --- a/wiring/inc/spark_wiring_cellular_printable.h +++ b/wiring/inc/spark_wiring_cellular_printable.h @@ -36,17 +36,10 @@ */ class CellularSignal : public particle::Signal, public Printable { public: - int rssi __attribute__((deprecated("Use getStrengthValue() instead"))) = 0; - int qual __attribute__((deprecated("Use getQualityValue() instead"))) = 0; - -// TODO: remove once rssi/qual are removed -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" CellularSignal() {} CellularSignal(const cellular_signal_t& sig); virtual ~CellularSignal() {}; CellularSignal(const CellularSignal&) = default; -#pragma GCC diagnostic pop bool fromHalCellularSignal(const cellular_signal_t& sig); diff --git a/wiring/src/spark_wiring_cellular.cpp b/wiring/src/spark_wiring_cellular.cpp index d4f5ca01f9..653b35185b 100644 --- a/wiring/src/spark_wiring_cellular.cpp +++ b/wiring/src/spark_wiring_cellular.cpp @@ -26,30 +26,18 @@ namespace spark { CellularSignal CellularClass::RSSI() { -// TODO: remove once rssi/qual are removed -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" CellularSignal sig; if (!network_ready(*this, 0, NULL)) { - sig.rssi = 0; return sig; } - CellularSignalHal sig_hal = {0}; cellular_signal_t sigext = {0}; sigext.size = sizeof(sigext); - if (cellular_signal(&sig_hal, &sigext) != 0) { - sig.rssi = 1; + if (cellular_signal(&sigext) != 0) { return sig; } - sig.rssi = sig_hal.rssi; - sig.qual = sig_hal.qual; - if (sig.rssi == 0) { - sig.rssi = 2; - } sig.fromHalCellularSignal(sigext); return sig; -#pragma GCC diagnostic pop } CellularDataHal data_hal; diff --git a/wiring/src/spark_wiring_cellular_printable.cpp b/wiring/src/spark_wiring_cellular_printable.cpp index bdbd884567..b0df2c9e22 100644 --- a/wiring/src/spark_wiring_cellular_printable.cpp +++ b/wiring/src/spark_wiring_cellular_printable.cpp @@ -24,9 +24,6 @@ #if Wiring_Cellular || defined(UNIT_TEST) -// TODO: remove once rssi/qual are removed -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" CellularSignal::CellularSignal(const cellular_signal_t& sig) : sig_(sig) { @@ -37,7 +34,6 @@ bool CellularSignal::fromHalCellularSignal(const cellular_signal_t& sig) sig_ = sig; return true; } -#pragma GCC diagnostic pop hal_net_access_tech_t CellularSignal::getAccessTechnology() const { @@ -80,18 +76,14 @@ float CellularSignal::getQualityValue() const return 0.0f; } -// TODO: remove once rssi/qual are removed -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" size_t CellularSignal::printTo(Print& p) const { size_t n = 0; - n += p.print((*this).rssi, DEC); + n += p.print((*this).getStrengthValue(), (int)2); n += p.print(','); - n += p.print((*this).qual, DEC); + n += p.print((*this).getQualityValue(), (int)2); return n; } -#pragma GCC diagnostic pop size_t CellularData::printTo(Print& p) const { From 1ff43ff92b70a7738129591c6fb2345e5318e679 Mon Sep 17 00:00:00 2001 From: keeramis Date: Wed, 7 Oct 2020 22:09:31 -0500 Subject: [PATCH 2/6] fix unit tests --- hal/inc/net_hal.h | 4 ++-- test/unit_tests/cellular/cellular.cpp | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/hal/inc/net_hal.h b/hal/inc/net_hal.h index 8a35891852..b18e612f2a 100644 --- a/hal/inc/net_hal.h +++ b/hal/inc/net_hal.h @@ -47,8 +47,8 @@ typedef enum { NET_ACCESS_TECHNOLOGY_UTRAN = 4, NET_ACCESS_TECHNOLOGY_WCDMA = 4, NET_ACCESS_TECHNOLOGY_CDMA = 5, - NET_ACCESS_TECHNOLOGY_LTE = 6, - NET_ACCESS_TECHNOLOGY_IEEE802154 = 7, + NET_ACCESS_TECHNOLOGY_IEEE802154 = 6, + NET_ACCESS_TECHNOLOGY_LTE = 7, NET_ACCESS_TECHNOLOGY_LTE_CAT_M1 = 8, NET_ACCESS_TECHNOLOGY_LTE_CAT_NB1 = 9, } hal_net_access_tech_t; diff --git a/test/unit_tests/cellular/cellular.cpp b/test/unit_tests/cellular/cellular.cpp index 8d881cab98..a1fda03875 100644 --- a/test/unit_tests/cellular/cellular.cpp +++ b/test/unit_tests/cellular/cellular.cpp @@ -179,6 +179,8 @@ TEST_CASE("cellular_signal()") { REQUIRE(sig.rat == NET_ACCESS_TECHNOLOGY_NONE); REQUIRE(sig.strength == 0); REQUIRE(sig.quality == 0); + REQUIRE(sig.rssi == std::numeric_limits::min()); + REQUIRE(sig.qual == std::numeric_limits::min()); SECTION("CellularSignal") { CellularSignal cs; @@ -681,12 +683,16 @@ TEST_CASE("cellular_printable") { SECTION("CellularSignal::printTo") { CellularSignal cs; - cs.getStrengthValue() = -83.70; - cs.getQualityValue = -7.00; + cellular_signal_t sig = {0}; + sig.size = sizeof(sig); + sig.rssi = -9000; + sig.qual = -1400; + sig.rat = 7; + cs.fromHalCellularSignal(sig); ser.print(cs); // printf("%s", output); - REQUIRE(strncmp(output, "-80.00,-7.00", sizeof(output)) == 0); + REQUIRE(strncmp(output, "-90.00,-14.00", sizeof(output)) == 0); } SECTION("CellularData::printTo") { From c01409a1a9b5ce84b99104716565a19dc4808d2e Mon Sep 17 00:00:00 2001 From: keeramis Date: Fri, 9 Oct 2020 17:14:33 -0500 Subject: [PATCH 3/6] Fix implementation in dynalib --- hal/inc/cellular_hal.h | 2 +- hal/inc/hal_dynalib_cellular.h | 2 +- hal/network/ncp/cellular/cellular_hal.cpp | 6 +++++- hal/src/electron/cellular_hal.cpp | 4 ++-- wiring/src/spark_wiring_cellular.cpp | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/hal/inc/cellular_hal.h b/hal/inc/cellular_hal.h index 282627c718..f33cdb83f5 100644 --- a/hal/inc/cellular_hal.h +++ b/hal/inc/cellular_hal.h @@ -159,7 +159,7 @@ void cellular_cancel(bool cancel, bool calledFromISR, void* reserved); /** * Retrieve cellular signal strength info */ -cellular_result_t cellular_signal(cellular_signal_t* reserved); +cellular_result_t cellular_signal(void* deprecated, cellular_signal_t* reserved); /** * Send an AT command and wait for response, optionally specify a callback function to parse the results diff --git a/hal/inc/hal_dynalib_cellular.h b/hal/inc/hal_dynalib_cellular.h index 8f9b2c5da5..d5de22bc9b 100644 --- a/hal/inc/hal_dynalib_cellular.h +++ b/hal/inc/hal_dynalib_cellular.h @@ -54,7 +54,7 @@ DYNALIB_FN(13, hal_cellular, cellular_cancel, void(bool, bool, void*)) DYNALIB_FN(14, hal_cellular, HAL_NET_SetNetWatchDog, uint32_t(uint32_t)) DYNALIB_FN(15, hal_cellular, inet_gethostbyname, int(const char*, uint16_t, HAL_IPAddress*, network_interface_t, void*)) DYNALIB_FN(16, hal_cellular, inet_ping, int(const HAL_IPAddress*, network_interface_t, uint8_t, void*)) -DYNALIB_FN(17, hal_cellular, cellular_signal, cellular_result_t(cellular_signal_t*)) +DYNALIB_FN(17, hal_cellular, cellular_signal, cellular_result_t(void*, cellular_signal_t*)) DYNALIB_FN(18, hal_cellular, cellular_command, cellular_result_t(_CALLBACKPTR_MDM, void*, system_tick_t, const char*, ...)) DYNALIB_FN(19, hal_cellular, cellular_data_usage_set, cellular_result_t(CellularDataHal*,void*)) DYNALIB_FN(20, hal_cellular, cellular_data_usage_get, cellular_result_t(CellularDataHal*,void*)) diff --git a/hal/network/ncp/cellular/cellular_hal.cpp b/hal/network/ncp/cellular/cellular_hal.cpp index 0b8babe13b..d58796919c 100644 --- a/hal/network/ncp/cellular/cellular_hal.cpp +++ b/hal/network/ncp/cellular/cellular_hal.cpp @@ -294,7 +294,11 @@ bool cellular_sim_ready(void* reserved) { void cellular_cancel(bool cancel, bool calledFromISR, void* reserved) { } -int cellular_signal(cellular_signal_t* signalExt) { +int cellular_signal(void* deprecated, cellular_signal_t* signalExt) { + if (deprecated != nullptr && signalExt == nullptr) { + return SYSTEM_ERROR_INVALID_ARGUMENT; + } + const auto mgr = cellularNetworkManager(); CHECK_TRUE(mgr, SYSTEM_ERROR_UNKNOWN); const auto client = mgr->ncpClient(); diff --git a/hal/src/electron/cellular_hal.cpp b/hal/src/electron/cellular_hal.cpp index aff57c2d72..26e7aadbf1 100644 --- a/hal/src/electron/cellular_hal.cpp +++ b/hal/src/electron/cellular_hal.cpp @@ -227,9 +227,9 @@ void cellular_cancel(bool cancel, bool calledFromISR, void*) } } -cellular_result_t cellular_signal(cellular_signal_t* signalext) +cellular_result_t cellular_signal(void* deprecated, cellular_signal_t* signalext) { - if (signalext == nullptr) { + if (deprecated != nullptr && signalext == nullptr) { return SYSTEM_ERROR_INVALID_ARGUMENT; } diff --git a/wiring/src/spark_wiring_cellular.cpp b/wiring/src/spark_wiring_cellular.cpp index 653b35185b..9d86894070 100644 --- a/wiring/src/spark_wiring_cellular.cpp +++ b/wiring/src/spark_wiring_cellular.cpp @@ -33,7 +33,7 @@ namespace spark { cellular_signal_t sigext = {0}; sigext.size = sizeof(sigext); - if (cellular_signal(&sigext) != 0) { + if (cellular_signal(nullptr, &sigext) != 0) { return sig; } sig.fromHalCellularSignal(sigext); From 09643505908909ed29e449c4f394cd3197e525a4 Mon Sep 17 00:00:00 2001 From: keeramis Date: Thu, 15 Oct 2020 15:33:46 -0500 Subject: [PATCH 4/6] Add API calls to check validity of signal values --- hal/inc/net_hal.h | 4 +- wiring/inc/spark_wiring_cellular_printable.h | 7 ++++ .../src/spark_wiring_cellular_printable.cpp | 40 ++++++++++++++++++- wiring/src/spark_wiring_print.cpp | 9 +++-- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/hal/inc/net_hal.h b/hal/inc/net_hal.h index b18e612f2a..8a35891852 100644 --- a/hal/inc/net_hal.h +++ b/hal/inc/net_hal.h @@ -47,8 +47,8 @@ typedef enum { NET_ACCESS_TECHNOLOGY_UTRAN = 4, NET_ACCESS_TECHNOLOGY_WCDMA = 4, NET_ACCESS_TECHNOLOGY_CDMA = 5, - NET_ACCESS_TECHNOLOGY_IEEE802154 = 6, - NET_ACCESS_TECHNOLOGY_LTE = 7, + NET_ACCESS_TECHNOLOGY_LTE = 6, + NET_ACCESS_TECHNOLOGY_IEEE802154 = 7, NET_ACCESS_TECHNOLOGY_LTE_CAT_M1 = 8, NET_ACCESS_TECHNOLOGY_LTE_CAT_NB1 = 9, } hal_net_access_tech_t; diff --git a/wiring/inc/spark_wiring_cellular_printable.h b/wiring/inc/spark_wiring_cellular_printable.h index 73a49465f1..cd8ce7e07c 100644 --- a/wiring/inc/spark_wiring_cellular_printable.h +++ b/wiring/inc/spark_wiring_cellular_printable.h @@ -51,6 +51,13 @@ class CellularSignal : public particle::Signal, public Printable { virtual size_t printTo(Print& p) const; + virtual bool isAccessTechnologyValid() const; + virtual bool isSignalStrengthValid() const; + virtual bool isSignalQualityValid() const; + virtual bool isSignalQualitySupported() const; + virtual bool isValid() const; + virtual operator bool() const; + private: cellular_signal_t sig_ = {0}; }; diff --git a/wiring/src/spark_wiring_cellular_printable.cpp b/wiring/src/spark_wiring_cellular_printable.cpp index b0df2c9e22..8a4e2e319f 100644 --- a/wiring/src/spark_wiring_cellular_printable.cpp +++ b/wiring/src/spark_wiring_cellular_printable.cpp @@ -20,6 +20,7 @@ #include "spark_wiring_cellular_printable.h" #include "spark_wiring_print.h" #include "string.h" +#include #include #if Wiring_Cellular || defined(UNIT_TEST) @@ -79,12 +80,47 @@ float CellularSignal::getQualityValue() const size_t CellularSignal::printTo(Print& p) const { size_t n = 0; - n += p.print((*this).getStrengthValue(), (int)2); + n += p.print(this->getStrengthValue(), 2); n += p.print(','); - n += p.print((*this).getQualityValue(), (int)2); + n += p.print(this->getQualityValue(), 2); return n; } +bool CellularSignal::isAccessTechnologyValid() const +{ + return (sig_.rat != NET_ACCESS_TECHNOLOGY_NONE); +} + +bool CellularSignal::isSignalStrengthValid() const +{ + return (sig_.rssi != std::numeric_limits::min()); +} + +bool CellularSignal::isSignalQualityValid() const +{ + return (sig_.qual != std::numeric_limits::min()); +} + +bool CellularSignal::isSignalQualitySupported() const +{ +#if (PLATFORM_ID == PLATFORM_ELECTRON_PRODUCTION) + return (sig_.rat == NET_ACCESS_TECHNOLOGY_GSM) ? false : true; +#else + return true; +#endif +} + +bool CellularSignal::isValid() const +{ + return (isAccessTechnologyValid() && isSignalStrengthValid() && + (isSignalQualitySupported() ? isSignalQualityValid() : true)); +} + +CellularSignal::operator bool() const +{ + return isValid(); +} + size_t CellularData::printTo(Print& p) const { size_t n = 0; diff --git a/wiring/src/spark_wiring_print.cpp b/wiring/src/spark_wiring_print.cpp index 7b83c94aef..031a8f3566 100644 --- a/wiring/src/spark_wiring_print.cpp +++ b/wiring/src/spark_wiring_print.cpp @@ -170,10 +170,11 @@ size_t Print::printFloat(double number, uint8_t digits) { size_t n = 0; - if (isnan(number)) return print("nan"); - if (isinf(number)) return print("inf"); - if (number > 4294967040.0) return print ("ovf"); // constant determined empirically - if (number <-4294967040.0) return print ("ovf"); // constant determined empirically + // FIXME: Trade off to save memory + //if (isnan(number)) return print("nan"); + //if (isinf(number)) return print("inf"); + //if (number > 4294967040.0) return print ("ovf"); // constant determined empirically + //if (number <-4294967040.0) return print ("ovf"); // constant determined empirically // Handle negative numbers if (number < 0.0) From 129fdccc71dc86d258bbc514e5d62a428f45d735 Mon Sep 17 00:00:00 2001 From: keeramis Date: Fri, 6 Nov 2020 13:39:37 -0600 Subject: [PATCH 5/6] Fix isValid(), add alias for RSSI class --- wiring/inc/spark_wiring_cellular.h | 1 + wiring/inc/spark_wiring_cellular_printable.h | 4 --- wiring/src/spark_wiring_cellular.cpp | 4 +++ .../src/spark_wiring_cellular_printable.cpp | 29 ++++--------------- wiring/src/spark_wiring_print.cpp | 9 +++--- 5 files changed, 14 insertions(+), 33 deletions(-) diff --git a/wiring/inc/spark_wiring_cellular.h b/wiring/inc/spark_wiring_cellular.h index b5bf3241e5..51c58c4e97 100644 --- a/wiring/inc/spark_wiring_cellular.h +++ b/wiring/inc/spark_wiring_cellular.h @@ -102,6 +102,7 @@ class CellularClass : public NetworkClass } CellularSignal RSSI(); + CellularSignal getSignal(); // Alias for Cellular.RSSI() bool getDataUsage(CellularData &data_get); bool setDataUsage(CellularData &data_set); diff --git a/wiring/inc/spark_wiring_cellular_printable.h b/wiring/inc/spark_wiring_cellular_printable.h index cd8ce7e07c..78a9708340 100644 --- a/wiring/inc/spark_wiring_cellular_printable.h +++ b/wiring/inc/spark_wiring_cellular_printable.h @@ -51,10 +51,6 @@ class CellularSignal : public particle::Signal, public Printable { virtual size_t printTo(Print& p) const; - virtual bool isAccessTechnologyValid() const; - virtual bool isSignalStrengthValid() const; - virtual bool isSignalQualityValid() const; - virtual bool isSignalQualitySupported() const; virtual bool isValid() const; virtual operator bool() const; diff --git a/wiring/src/spark_wiring_cellular.cpp b/wiring/src/spark_wiring_cellular.cpp index 9d86894070..9e4ffda010 100644 --- a/wiring/src/spark_wiring_cellular.cpp +++ b/wiring/src/spark_wiring_cellular.cpp @@ -40,6 +40,10 @@ namespace spark { return sig; } + CellularSignal CellularClass::getSignal() { + return this->RSSI(); + } + CellularDataHal data_hal; bool CellularClass::getDataUsage(CellularData &data_get) { diff --git a/wiring/src/spark_wiring_cellular_printable.cpp b/wiring/src/spark_wiring_cellular_printable.cpp index 8a4e2e319f..3809fa1b4f 100644 --- a/wiring/src/spark_wiring_cellular_printable.cpp +++ b/wiring/src/spark_wiring_cellular_printable.cpp @@ -86,36 +86,17 @@ size_t CellularSignal::printTo(Print& p) const return n; } -bool CellularSignal::isAccessTechnologyValid() const -{ - return (sig_.rat != NET_ACCESS_TECHNOLOGY_NONE); -} - -bool CellularSignal::isSignalStrengthValid() const -{ - return (sig_.rssi != std::numeric_limits::min()); -} - -bool CellularSignal::isSignalQualityValid() const -{ - return (sig_.qual != std::numeric_limits::min()); -} - -bool CellularSignal::isSignalQualitySupported() const +bool CellularSignal::isValid() const { + return (sig_.rat != NET_ACCESS_TECHNOLOGY_NONE && + sig_.rssi != std::numeric_limits::min() && #if (PLATFORM_ID == PLATFORM_ELECTRON_PRODUCTION) - return (sig_.rat == NET_ACCESS_TECHNOLOGY_GSM) ? false : true; + (sig_.rat == NET_ACCESS_TECHNOLOGY_GSM) ? true : (sig_.qual != std::numeric_limits::min())); #else - return true; + sig_.qual != std::numeric_limits::min()); #endif } -bool CellularSignal::isValid() const -{ - return (isAccessTechnologyValid() && isSignalStrengthValid() && - (isSignalQualitySupported() ? isSignalQualityValid() : true)); -} - CellularSignal::operator bool() const { return isValid(); diff --git a/wiring/src/spark_wiring_print.cpp b/wiring/src/spark_wiring_print.cpp index 031a8f3566..7b83c94aef 100644 --- a/wiring/src/spark_wiring_print.cpp +++ b/wiring/src/spark_wiring_print.cpp @@ -170,11 +170,10 @@ size_t Print::printFloat(double number, uint8_t digits) { size_t n = 0; - // FIXME: Trade off to save memory - //if (isnan(number)) return print("nan"); - //if (isinf(number)) return print("inf"); - //if (number > 4294967040.0) return print ("ovf"); // constant determined empirically - //if (number <-4294967040.0) return print ("ovf"); // constant determined empirically + if (isnan(number)) return print("nan"); + if (isinf(number)) return print("inf"); + if (number > 4294967040.0) return print ("ovf"); // constant determined empirically + if (number <-4294967040.0) return print ("ovf"); // constant determined empirically // Handle negative numbers if (number < 0.0) From dc4a00a7a825b6acfad744692b75f594812a0fda Mon Sep 17 00:00:00 2001 From: keeramis Date: Tue, 10 Nov 2020 16:57:56 -0600 Subject: [PATCH 6/6] Make getSignal inline, add comments --- wiring/inc/spark_wiring_cellular.h | 5 ++++- wiring/src/spark_wiring_cellular.cpp | 4 ---- wiring/src/spark_wiring_cellular_printable.cpp | 6 +++++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/wiring/inc/spark_wiring_cellular.h b/wiring/inc/spark_wiring_cellular.h index 51c58c4e97..be6fcd68f0 100644 --- a/wiring/inc/spark_wiring_cellular.h +++ b/wiring/inc/spark_wiring_cellular.h @@ -102,7 +102,10 @@ class CellularClass : public NetworkClass } CellularSignal RSSI(); - CellularSignal getSignal(); // Alias for Cellular.RSSI() + + inline const CellularSignal getSignal() { + return RSSI(); + } bool getDataUsage(CellularData &data_get); bool setDataUsage(CellularData &data_set); diff --git a/wiring/src/spark_wiring_cellular.cpp b/wiring/src/spark_wiring_cellular.cpp index 9e4ffda010..9d86894070 100644 --- a/wiring/src/spark_wiring_cellular.cpp +++ b/wiring/src/spark_wiring_cellular.cpp @@ -40,10 +40,6 @@ namespace spark { return sig; } - CellularSignal CellularClass::getSignal() { - return this->RSSI(); - } - CellularDataHal data_hal; bool CellularClass::getDataUsage(CellularData &data_get) { diff --git a/wiring/src/spark_wiring_cellular_printable.cpp b/wiring/src/spark_wiring_cellular_printable.cpp index 3809fa1b4f..bfd7046cc6 100644 --- a/wiring/src/spark_wiring_cellular_printable.cpp +++ b/wiring/src/spark_wiring_cellular_printable.cpp @@ -91,7 +91,11 @@ bool CellularSignal::isValid() const return (sig_.rat != NET_ACCESS_TECHNOLOGY_NONE && sig_.rssi != std::numeric_limits::min() && #if (PLATFORM_ID == PLATFORM_ELECTRON_PRODUCTION) - (sig_.rat == NET_ACCESS_TECHNOLOGY_GSM) ? true : (sig_.qual != std::numeric_limits::min())); + // U-blox GSM radios may not always support quality as it depends on the packet switching mode + // at the time of network connection, which is not possible to query. For now, we will return "true" + // for GSM Electrons and will not check if quality is actually supported / valid. + // Hence, `isValid()` can return "true" for invalid signal value. To add a note in docs. + (sig_.rat == NET_ACCESS_TECHNOLOGY_GSM || sig_.qual != std::numeric_limits::min())); #else sig_.qual != std::numeric_limits::min()); #endif