diff --git a/hal/network/lwip/cellular/pppncpnetif.cpp b/hal/network/lwip/cellular/pppncpnetif.cpp index b2561f33d6..5f02f3de4e 100644 --- a/hal/network/lwip/cellular/pppncpnetif.cpp +++ b/hal/network/lwip/cellular/pppncpnetif.cpp @@ -119,29 +119,32 @@ void PppNcpNetif::loop(void* arg) { self->celMan_->ncpClient()->enable(); // Make sure the client is enabled os_semaphore_take(self->netifSemaphore_, timeout, false); - if (self->expectedNcpState_ == NcpState::ON && self->celMan_->ncpClient()->ncpState() != NcpState::ON) { - auto r = self->celMan_->ncpClient()->on(); - if (r != SYSTEM_ERROR_NONE && r != SYSTEM_ERROR_ALREADY_EXISTS) { - LOG(ERROR, "Failed to initialize cellular NCP client: %d", r); + // We shouldn't be enforcing state on boot! + if (self->lastNetifEvent_ != NetifEvent::None) { + if (self->expectedNcpState_ == NcpState::ON && self->celMan_->ncpClient()->ncpState() != NcpState::ON) { + auto r = self->celMan_->ncpClient()->on(); + if (r != SYSTEM_ERROR_NONE && r != SYSTEM_ERROR_ALREADY_EXISTS) { + LOG(ERROR, "Failed to initialize cellular NCP client: %d", r); + } + } + if (self->expectedConnectionState_ == NcpConnectionState::CONNECTED && + self->celMan_->ncpClient()->connectionState() == NcpConnectionState::DISCONNECTED && + self->celMan_->ncpClient()->ncpState() == NcpState::ON) { + self->upImpl(); } - } - if (self->expectedConnectionState_ == NcpConnectionState::CONNECTED && - self->celMan_->ncpClient()->connectionState() == NcpConnectionState::DISCONNECTED && - self->celMan_->ncpClient()->ncpState() == NcpState::ON) { - self->upImpl(); - } - if (self->expectedConnectionState_ == NcpConnectionState::DISCONNECTED && - self->celMan_->ncpClient()->connectionState() != NcpConnectionState::DISCONNECTED) { - self->downImpl(); - } - if (self->expectedNcpState_ == NcpState::OFF && self->celMan_->ncpClient()->connectionState() == NcpConnectionState::DISCONNECTED) { - if_power_state_t pwrState = IF_POWER_STATE_NONE; - self->getPowerState(&pwrState); - if (pwrState == IF_POWER_STATE_UP) { - auto r = self->celMan_->ncpClient()->off(); - if (r != SYSTEM_ERROR_NONE) { - LOG(ERROR, "Failed to turn off NCP client: %d", r); + if (self->expectedConnectionState_ == NcpConnectionState::DISCONNECTED && + self->celMan_->ncpClient()->connectionState() != NcpConnectionState::DISCONNECTED) { + self->downImpl(); + } + if (self->expectedNcpState_ == NcpState::OFF && self->celMan_->ncpClient()->connectionState() == NcpConnectionState::DISCONNECTED) { + if_power_state_t pwrState = IF_POWER_STATE_NONE; + self->getPowerState(&pwrState); + if (pwrState == IF_POWER_STATE_UP) { + auto r = self->celMan_->ncpClient()->off(); + if (r != SYSTEM_ERROR_NONE) { + LOG(ERROR, "Failed to turn off NCP client: %d", r); + } } } } diff --git a/hal/network/lwip/esp32/esp32ncpnetif.cpp b/hal/network/lwip/esp32/esp32ncpnetif.cpp index 4b9da14586..494ceec3c8 100644 --- a/hal/network/lwip/esp32/esp32ncpnetif.cpp +++ b/hal/network/lwip/esp32/esp32ncpnetif.cpp @@ -172,29 +172,32 @@ void Esp32NcpNetif::loop(void* arg) { self->wifiMan_->ncpClient()->enable(); // Make sure the client is enabled os_semaphore_take(self->netifSemaphore_, timeout, false); - if (self->expectedNcpState_ == NcpState::ON && self->wifiMan_->ncpClient()->ncpState() != NcpState::ON) { - auto r = self->wifiMan_->ncpClient()->on(); - if (r != SYSTEM_ERROR_NONE && r != SYSTEM_ERROR_ALREADY_EXISTS) { - LOG(ERROR, "Failed to initialize cellular NCP client: %d", r); + // We shouldn't be enforcing state on boot! + if (self->lastNetifEvent_ != NetifEvent::None) { + if (self->expectedNcpState_ == NcpState::ON && self->wifiMan_->ncpClient()->ncpState() != NcpState::ON) { + auto r = self->wifiMan_->ncpClient()->on(); + if (r != SYSTEM_ERROR_NONE && r != SYSTEM_ERROR_ALREADY_EXISTS) { + LOG(ERROR, "Failed to initialize cellular NCP client: %d", r); + } + } + if (self->expectedConnectionState_ == NcpConnectionState::CONNECTED && + self->wifiMan_->ncpClient()->connectionState() == NcpConnectionState::DISCONNECTED && + self->wifiMan_->ncpClient()->ncpState() == NcpState::ON) { + self->upImpl(); } - } - if (self->expectedConnectionState_ == NcpConnectionState::CONNECTED && - self->wifiMan_->ncpClient()->connectionState() == NcpConnectionState::DISCONNECTED && - self->wifiMan_->ncpClient()->ncpState() == NcpState::ON) { - self->upImpl(); - } - if (self->expectedConnectionState_ == NcpConnectionState::DISCONNECTED && - self->wifiMan_->ncpClient()->connectionState() != NcpConnectionState::DISCONNECTED) { - self->downImpl(); - } - if (self->expectedNcpState_ == NcpState::OFF && self->wifiMan_->ncpClient()->connectionState() == NcpConnectionState::DISCONNECTED) { - if_power_state_t pwrState = IF_POWER_STATE_NONE; - self->getPowerState(&pwrState); - if (pwrState == IF_POWER_STATE_UP) { - auto r = self->wifiMan_->ncpClient()->off(); - if (r != SYSTEM_ERROR_NONE) { - LOG(ERROR, "Failed to turn off NCP client: %d", r); + if (self->expectedConnectionState_ == NcpConnectionState::DISCONNECTED && + self->wifiMan_->ncpClient()->connectionState() != NcpConnectionState::DISCONNECTED) { + self->downImpl(); + } + if (self->expectedNcpState_ == NcpState::OFF && self->wifiMan_->ncpClient()->connectionState() == NcpConnectionState::DISCONNECTED) { + if_power_state_t pwrState = IF_POWER_STATE_NONE; + self->getPowerState(&pwrState); + if (pwrState == IF_POWER_STATE_UP) { + auto r = self->wifiMan_->ncpClient()->off(); + if (r != SYSTEM_ERROR_NONE) { + LOG(ERROR, "Failed to turn off NCP client: %d", r); + } } } } diff --git a/hal/network/ncp/wifi/platform_ncp_update.cpp b/hal/network/ncp/wifi/platform_ncp_update.cpp index cbe4413407..54fb506ca2 100644 --- a/hal/network/ncp/wifi/platform_ncp_update.cpp +++ b/hal/network/ncp/wifi/platform_ncp_update.cpp @@ -15,10 +15,13 @@ * License along with this library; if not, see . */ +#undef LOG_COMPILE_TIME_LEVEL + #include "platform_ncp.h" #include "network/ncp/wifi/ncp.h" #include "network/ncp/wifi/wifi_network_manager.h" #include "network/ncp/wifi/wifi_ncp_client.h" +#include "network/ncp/ncp_client.h" #include "led_service.h" #include "check.h" #include "scope_guard.h" @@ -26,9 +29,12 @@ #include "stream.h" #include "debug.h" #include "ota_flash_hal_impl.h" +#include "system_cache.h" #include +namespace { + class OtaUpdateSourceStream : public particle::InputStream { public: OtaUpdateSourceStream(const uint8_t* buffer, size_t length) : buffer(buffer), remaining(length) {} @@ -80,12 +86,54 @@ class OtaUpdateSourceStream : public particle::InputStream { size_t remaining; }; +int invalidateWifiNcpVersionCache() { +#if HAL_PLATFORM_NCP_COUNT > 1 + using namespace particle::services; + // Cache will be updated by the NCP client itself + LOG(TRACE, "Invalidating cached ESP32 NCP firmware version"); + return SystemCache::instance().del(SystemCacheKey::WIFI_NCP_FIRMWARE_VERSION); +#else + return 0; +#endif // HAL_PLATFORM_NCP_COUNT > 1 +} + +int getWifiNcpFirmwareVersion(uint16_t* ncpVersion) { + uint16_t version = 0; +#if HAL_PLATFORM_NCP_COUNT > 1 + using namespace particle::services; + int res = SystemCache::instance().get(SystemCacheKey::WIFI_NCP_FIRMWARE_VERSION, &version, sizeof(version)); + if (res == sizeof(version)) { + LOG(TRACE, "Cached ESP32 NCP firmware version: %d", (int)version); + *ncpVersion = version; + return 0; + } + + if (res >= 0) { + invalidateWifiNcpVersionCache(); + } +#endif // HAL_PLATFORM_NCP_COUNT > 1 + + // Not present in cache or caching not supported, call into NCP client + const auto ncpClient = particle::wifiNetworkManager()->ncpClient(); + SPARK_ASSERT(ncpClient); + const particle::NcpClientLock lock(ncpClient); + CHECK(ncpClient->on()); + CHECK(ncpClient->getFirmwareModuleVersion(&version)); + *ncpVersion = version; + + return 0; +} + +} // anonymous + // FIXME: This function accesses the module info via XIP and may fail to parse it correctly under // some not entirely clear circumstances. Disabling compiler optimizations helps to work around // the problem __attribute__((optimize("O0"))) int platform_ncp_update_module(const hal_module_t* module) { const auto ncpClient = particle::wifiNetworkManager()->ncpClient(); SPARK_ASSERT(ncpClient); + // Holding a lock for the whole duration of the operation, otherwise the netif may potentially poweroff the NCP + const particle::NcpClientLock lock(ncpClient); CHECK(ncpClient->on()); // we pass only the actual binary after the module info and up to the suffix const uint8_t* start = (const uint8_t*)module->info; @@ -99,6 +147,7 @@ __attribute__((optimize("O0"))) int platform_ncp_update_module(const hal_module_ if (r == 0) { LOG(INFO, "Updating ESP32 firmware from version %d to version %d", version, module->info->module_version); } + invalidateWifiNcpVersionCache(); r = ncpClient->updateFirmware(&moduleStream, length); LED_On(PARTICLE_LED_RGB); CHECK(r); @@ -114,14 +163,10 @@ int platform_ncp_fetch_module_info(hal_system_info_t* sys_info, bool create) { hal_module_t* module = sys_info->modules + i; if (!memcmp(&module->bounds, &module_ncp_mono, sizeof(module_ncp_mono))) { if (create) { - const auto ncpClient = particle::wifiNetworkManager()->ncpClient(); - SPARK_ASSERT(ncpClient); - CHECK(ncpClient->on()); - uint16_t version; - int error = ncpClient->getFirmwareModuleVersion(&version); - if (error) { - version = 0; - } + uint16_t version = 0; + // Defaults to zero in case of failure + getWifiNcpFirmwareVersion(&version); + // todo - we could augment the getFirmwareModuleVersion command to retrieve more details auto info = new module_info_t(); CHECK_TRUE(info, SYSTEM_ERROR_NO_MEMORY); diff --git a/hal/network/ncp_client/esp32/esp32_ncp_client.cpp b/hal/network/ncp_client/esp32/esp32_ncp_client.cpp index 2df1f1bfb3..a53dae87f1 100644 --- a/hal/network/ncp_client/esp32/esp32_ncp_client.cpp +++ b/hal/network/ncp_client/esp32/esp32_ncp_client.cpp @@ -35,6 +35,7 @@ LOG_SOURCE_CATEGORY("ncp.esp32.client"); #include "check.h" #include +#include "system_cache.h" #define CHECK_PARSER(_expr) \ ({ \ @@ -75,6 +76,21 @@ size_t espEscape(const char* src, char* dest, size_t destSize) { return escape(src, ",\"\\", '\\', dest, destSize); } +int updateCachedNcpFirmwareVersion(uint16_t version) { +#if HAL_PLATFORM_NCP_COUNT > 1 + using namespace particle::services; + + uint16_t cached = 0; + int r = SystemCache::instance().get(SystemCacheKey::WIFI_NCP_FIRMWARE_VERSION, &cached, sizeof(cached)); + if (r == sizeof(cached) && cached == version) { + return 0; + } + return SystemCache::instance().set(SystemCacheKey::WIFI_NCP_FIRMWARE_VERSION, &version, sizeof(version)); +#else + return 0; +#endif // HAL_PLATFORM_NCP_COUNT > 0 +} + const auto ESP32_NCP_MAX_MUXER_FRAME_SIZE = 1536; const auto ESP32_NCP_KEEPALIVE_PERIOD = 5000; // milliseconds const auto ESP32_NCP_KEEPALIVE_MAX_MISSED = 5; @@ -274,7 +290,11 @@ int Esp32NcpClient::getFirmwareVersionString(char* buf, size_t size) { int Esp32NcpClient::getFirmwareModuleVersion(uint16_t* ver) { const NcpClientLock lock(this); CHECK(checkParser()); - return getFirmwareModuleVersionImpl(ver); + const int r = getFirmwareModuleVersionImpl(ver); + if (!r) { + updateCachedNcpFirmwareVersion(*ver); + } + return r; } int Esp32NcpClient::getFirmwareModuleVersionImpl(uint16_t* ver) { diff --git a/services/inc/system_cache.h b/services/inc/system_cache.h new file mode 100644 index 0000000000..c13afaea0c --- /dev/null +++ b/services/inc/system_cache.h @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2020 Particle Industries, Inc. All rights reserved. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation, either + * version 3 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + */ + +#pragma once + +#include "tlv_file.h" + +namespace particle { namespace services { + +enum class SystemCacheKey : uint16_t { + WIFI_NCP_FIRMWARE_VERSION = 0x0000 +}; + +class SystemCache { +public: + static SystemCache& instance(); + + // Just straight up proxying to TlvFile + // TODO: add support for multiple values for a key? + int get(SystemCacheKey key, void* value, size_t length); + int set(SystemCacheKey key, const void* value, size_t length); + int del(SystemCacheKey key); + + SystemCache(SystemCache const&) = delete; + SystemCache(SystemCache&&) = delete; + SystemCache& operator=(SystemCache const&) = delete; + SystemCache& operator=(SystemCache &&) = delete; + +protected: + SystemCache(); + +private: + settings::TlvFile tlv_; +}; + +} } // particle::service diff --git a/services/src/system_cache.cpp b/services/src/system_cache.cpp new file mode 100644 index 0000000000..64ededb776 --- /dev/null +++ b/services/src/system_cache.cpp @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2020 Particle Industries, Inc. All rights reserved. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation, either + * version 3 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + */ + +#include "hal_platform.h" + +#if HAL_PLATFORM_FILESYSTEM + +#include "system_cache.h" +#include "enumclass.h" +#include "check.h" + +namespace particle { namespace services { + +SystemCache::SystemCache() + : tlv_("/sys/cache.dat") { +} + +SystemCache& SystemCache::instance() { + static SystemCache cache; + cache.tlv_.init(); + return cache; +} + +int SystemCache::get(SystemCacheKey key, void* value, size_t length) { + return tlv_.get(to_underlying(key), (uint8_t*)value, length); +} + +int SystemCache::set(SystemCacheKey key, const void* value, size_t length) { + CHECK_TRUE(length <= sizeof(uint16_t), SYSTEM_ERROR_TOO_LARGE); + return tlv_.set(to_underlying(key), (const uint8_t*)value, length); +} + +int SystemCache::del(SystemCacheKey key) { + return tlv_.del(to_underlying(key)); +} + +} } // particle::services + +#endif // HAL_PLATFORM_FILESYSTEM diff --git a/system/src/control/cellular.cpp b/system/src/control/cellular.cpp index f8988eb5c1..94229de85f 100644 --- a/system/src/control/cellular.cpp +++ b/system/src/control/cellular.cpp @@ -116,6 +116,7 @@ int getIccid(ctrl_request* req) { CHECK_TRUE(cellMgr, SYSTEM_ERROR_UNKNOWN); const auto ncpClient = cellMgr->ncpClient(); CHECK_TRUE(ncpClient, SYSTEM_ERROR_UNKNOWN); + const NcpClientLock lock(ncpClient); CHECK(ncpClient->on()); char buf[32] = {}; CHECK(ncpClient->getIccid(buf, sizeof(buf))); diff --git a/system/src/control/config.cpp b/system/src/control/config.cpp index ac31cdbe7d..14eca5221c 100644 --- a/system/src/control/config.cpp +++ b/system/src/control/config.cpp @@ -108,6 +108,7 @@ int getNcpFirmwareVersion(ctrl_request* req) { CHECK_TRUE(wifiMgr, SYSTEM_ERROR_UNKNOWN); const auto ncpClient = wifiMgr->ncpClient(); CHECK_TRUE(ncpClient, SYSTEM_ERROR_UNKNOWN); + const NcpClientLock lock(ncpClient); CHECK(ncpClient->on()); char verStr[32] = {}; CHECK(ncpClient->getFirmwareVersionString(verStr, sizeof(verStr))); diff --git a/system/src/control/wifi_new.cpp b/system/src/control/wifi_new.cpp index 03f66613b8..6679d6128a 100644 --- a/system/src/control/wifi_new.cpp +++ b/system/src/control/wifi_new.cpp @@ -102,6 +102,7 @@ int joinNewNetwork(ctrl_request* req) { // Connect to the network const auto ncpClient = wifiMgr->ncpClient(); CHECK_TRUE(ncpClient, SYSTEM_ERROR_UNKNOWN); + const NcpClientLock lock(ncpClient); CHECK(ncpClient->on()); CHECK(ncpClient->disconnect()); CHECK(wifiMgr->connect(dSsid.data)); @@ -117,6 +118,7 @@ int joinKnownNetwork(ctrl_request* req) { CHECK_TRUE(wifiMgr, SYSTEM_ERROR_UNKNOWN); const auto ncpClient = wifiMgr->ncpClient(); CHECK_TRUE(ncpClient, SYSTEM_ERROR_UNKNOWN); + const NcpClientLock lock(ncpClient); CHECK(ncpClient->on()); CHECK(ncpClient->disconnect()); CHECK(wifiMgr->connect(dSsid.data)); @@ -195,6 +197,7 @@ int scanNetworks(ctrl_request* req) { CHECK_TRUE(wifiMgr, SYSTEM_ERROR_UNKNOWN); const auto ncpClient = wifiMgr->ncpClient(); CHECK_TRUE(ncpClient, SYSTEM_ERROR_UNKNOWN); + const NcpClientLock lock(ncpClient); CHECK(ncpClient->on()); // Scan for networks Vector networks;