From 4e25f05aa14390876a5573a470c5467d725f5d18 Mon Sep 17 00:00:00 2001 From: David Crocker Date: Wed, 28 Jun 2023 17:18:23 +0100 Subject: [PATCH] Fix to WiFiInterface for ESP32 --- src/Networking/ESP8266WiFi/WiFiInterface.cpp | 69 ++++++++++---------- src/Networking/ESP8266WiFi/WiFiInterface.h | 8 +-- 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/src/Networking/ESP8266WiFi/WiFiInterface.cpp b/src/Networking/ESP8266WiFi/WiFiInterface.cpp index b496dea0a3..1cf17ff9d1 100644 --- a/src/Networking/ESP8266WiFi/WiFiInterface.cpp +++ b/src/Networking/ESP8266WiFi/WiFiInterface.cpp @@ -131,7 +131,7 @@ static inline void DisableSpi() noexcept #endif } -static inline void EnableSpi() +static inline void EnableSpi() noexcept { #if SAME5x WiFiSpiSercom->SPI.CTRLA.reg |= SERCOM_SPI_CTRLA_ENABLE; @@ -142,7 +142,7 @@ static inline void EnableSpi() } // Clear the transmit and receive registers and put the SPI into slave mode, SPI mode 1 -static inline void ResetSpi() +static inline void ResetSpi() noexcept { #if SAME5x WiFiSpiSercom->SPI.CTRLA.reg |= SERCOM_SPI_CTRLA_SWRST; @@ -259,8 +259,8 @@ WiFiInterface::WiFiInterface(Platform& p) noexcept protocolEnabled[i] = (i == HttpProtocol); } - strcpy(actualSsid, "(unknown)"); - strcpy(wiFiServerVersion, "(unknown)"); + actualSsid.copy("(unknown)"); + wiFiServerVersion.copy("(unknown)"); #ifdef DUET3MINI SerialWiFiDevice = new AsyncSerial(WiFiUartSercomNumber, WiFiUartRxPad, 512, 512, SerialWiFiPortInit, SerialWiFiPortDeinit); @@ -277,21 +277,23 @@ WiFiInterface::WiFiInterface(Platform& p) noexcept // Otherwise the table will be allocated in RAM instead of flash, which wastes too much RAM. // Macro to build a standard lambda function that includes the necessary type conversions -#define OBJECT_MODEL_FUNC(_ret) OBJECT_MODEL_FUNC_BODY(WiFiInterface, _ret) +#define OBJECT_MODEL_FUNC(...) OBJECT_MODEL_FUNC_BODY(WiFiInterface, __VA_ARGS__) +#define OBJECT_MODEL_FUNC_IF(_condition,...) OBJECT_MODEL_FUNC_IF_BODY(WiFiInterface, _condition, __VA_ARGS__) constexpr ObjectModelTableEntry WiFiInterface::objectModelTable[] = { // These entries must be in alphabetical order - { "actualIP", OBJECT_MODEL_FUNC(self->ipAddress), ObjectModelEntryFlags::none }, - { "firmwareVersion", OBJECT_MODEL_FUNC(self->wiFiServerVersion), ObjectModelEntryFlags::none }, - { "gateway", OBJECT_MODEL_FUNC(self->gateway), ObjectModelEntryFlags::none }, - { "mac", OBJECT_MODEL_FUNC(self->macAddress), ObjectModelEntryFlags::none }, - { "state", OBJECT_MODEL_FUNC(self->GetStateName()), ObjectModelEntryFlags::none }, - { "subnet", OBJECT_MODEL_FUNC(self->netmask), ObjectModelEntryFlags::none }, - { "type", OBJECT_MODEL_FUNC_NOSELF("wifi"), ObjectModelEntryFlags::none }, + { "actualIP", OBJECT_MODEL_FUNC(self->ipAddress), ObjectModelEntryFlags::none }, + { "firmwareVersion", OBJECT_MODEL_FUNC(self->wiFiServerVersion.c_str()), ObjectModelEntryFlags::none }, + { "gateway", OBJECT_MODEL_FUNC(self->gateway), ObjectModelEntryFlags::none }, + { "mac", OBJECT_MODEL_FUNC_IF(self->GetState() == NetworkState::active, self->macAddress), ObjectModelEntryFlags::none }, + { "ssid", OBJECT_MODEL_FUNC_IF(self->GetState() == NetworkState::active, self->actualSsid.c_str()), ObjectModelEntryFlags::none }, + { "state", OBJECT_MODEL_FUNC(self->GetStateName()), ObjectModelEntryFlags::none }, + { "subnet", OBJECT_MODEL_FUNC(self->netmask), ObjectModelEntryFlags::none }, + { "type", OBJECT_MODEL_FUNC_NOSELF("wifi"), ObjectModelEntryFlags::none }, }; -constexpr uint8_t WiFiInterface::objectModelTableDescriptor[] = { 1, 7 }; +constexpr uint8_t WiFiInterface::objectModelTableDescriptor[] = { 1, 8 }; DEFINE_GET_OBJECT_MODEL_TABLE(WiFiInterface) @@ -518,7 +520,7 @@ GCodeResult WiFiInterface::GetNetworkState(const StringRef& reply) noexcept reply.cat(TranslateWiFiState(currentMode)); if (currentMode == WiFiState::connected || currentMode == WiFiState::runningAsAccessPoint) { - reply.catf("%s, IP address %s", actualSsid, IP4String(ipAddress).c_str()); + reply.catf("%s, IP address %s", actualSsid.c_str(), IP4String(ipAddress).c_str()); } break; default: @@ -671,9 +673,9 @@ void WiFiInterface::Spin() noexcept // Read the status to get the WiFi server version and MAC address Receiver status; int32_t rc = SendCommand(NetworkCommand::networkGetStatus, 0, 0, nullptr, 0, status); - if (rc > 0) + if (rc >= (int32_t)MinimumStatusResponseLength) { - SafeStrncpy(wiFiServerVersion, status.Value().versionText, ARRAY_SIZE(wiFiServerVersion)); + wiFiServerVersion.copy(status.Value().versionText); macAddress.SetFromBytes(status.Value().macAddress); // Set the hostname before anything else is done @@ -705,8 +707,7 @@ void WiFiInterface::Spin() noexcept } else { - // Something went wrong, maybe a bad firmware image was flashed - // Disable the WiFi chip again in this case + // Something went wrong, maybe a bad firmware image was flashed. Disable the WiFi chip again in this case Stop(); platform.MessageF(NetworkErrorMessage, "failed to initialise WiFi module: %s\n", TranslateWiFiResponse(rc)); } @@ -752,7 +753,7 @@ void WiFiInterface::Spin() noexcept } else if (requestedMode == WiFiState::connected) { - rslt = SendCommand(NetworkCommand::networkStartClient, 0, 0, 0, requestedSsid, SsidLength, nullptr, 0); + rslt = SendCommand(NetworkCommand::networkStartClient, 0, 0, 0, requestedSsid.Pointer(), requestedSsid.Capacity(), nullptr, 0); } else if (requestedMode == WiFiState::runningAsAccessPoint) { @@ -833,13 +834,13 @@ void WiFiInterface::Spin() noexcept if (SendCommand(NetworkCommand::networkGetStatus, 0, 0, nullptr, 0, status) > 0) { ipAddress.SetV4LittleEndian(status.Value().ipAddress); - SafeStrncpy(actualSsid, status.Value().ssid, SsidLength); + actualSsid.copy(status.Value().ssid); } InitSockets(); reconnectCount = 0; platform.MessageF(NetworkInfoMessage, "WiFi module is %s%s, IP address %s\n", TranslateWiFiState(currentMode), - actualSsid, + actualSsid.c_str(), IP4String(ipAddress).c_str()); } break; @@ -919,20 +920,20 @@ const char* WiFiInterface::TranslateEspResetReason(uint32_t reason) noexcept void WiFiInterface::Diagnostics(MessageType mtype) noexcept { - platform.MessageF(mtype, "= WiFi =\nNetwork state is %s\n", GetStateName()); - platform.MessageF(mtype, "WiFi module is %s\n", TranslateWiFiState(currentMode)); - platform.MessageF(mtype, "Failed messages: pending %u, notready %u, noresp %u\n", transferAlreadyPendingCount, readyTimeoutCount, responseTimeoutCount); - -#if 0 - // The underrun/overrun counters don't work at present - platform.MessageF(mtype, "SPI underruns %u, overruns %u\n", spiTxUnderruns, spiRxOverruns); -#endif + platform.MessageF(mtype, + "= WiFi =\nInterface state: %s\n" + "Module is %s\n" + "Failed messages: pending %u, notready %u, noresp %u\n", + GetStateName(), + TranslateWiFiState(currentMode), + transferAlreadyPendingCount, readyTimeoutCount, responseTimeoutCount + ); if (GetState() != NetworkState::disabled && GetState() != NetworkState::starting1 && GetState() != NetworkState::starting2) { Receiver status; status.Value().clockReg = 0xFFFFFFFF; // older WiFi firmware doesn't return this value, so preset it - if (SendCommand(NetworkCommand::networkGetStatus, 0, 0, nullptr, 0, status) > 0) + if (SendCommand(NetworkCommand::networkGetStatus, 0, 0, nullptr, 0, status) >= (int32_t)MinimumStatusResponseLength) { NetworkStatusResponse& r = status.Value(); r.versionText[ARRAY_UPB(r.versionText)] = 0; @@ -989,8 +990,7 @@ GCodeResult WiFiInterface::EnableInterface(int mode, const StringRef& ssid, cons : WiFiState::disabled; if (modeRequested == WiFiState::connected) { - memset(requestedSsid, 0, sizeof(requestedSsid)); - SafeStrncpy(requestedSsid, ssid.c_str(), ARRAY_SIZE(requestedSsid)); + requestedSsid.copy(ssid.c_str()); } if (activated) @@ -1806,7 +1806,7 @@ int32_t WiFiInterface::SendCommand(NetworkCommand cmd, SocketNumber socketNum, u bufferOut->hdr.param32 = param32; bufferOut->hdr.dataLength = (uint16_t)dataOutLength; bufferOut->hdr.dataBufferAvailable = (uint16_t)dataInLength; - if (dataOut != nullptr) + if (dataOut != nullptr && dataOut != &(bufferOut->data)) { memcpy(bufferOut->data, dataOut, dataOutLength); } @@ -1841,8 +1841,7 @@ int32_t WiFiInterface::SendCommand(NetworkCommand cmd, SocketNumber socketNum, u digitalWrite(SamTfrReadyPin, true); // Wait until the DMA transfer is complete, with timeout - // On factory reset, use the startup timeout, as it involves re-formatting the SPIFFS - // partition. + // On factory reset, use the startup timeout, as it involves re-formatting the SPIFFS partition. const uint32_t timeout = (cmd == NetworkCommand::networkFactoryReset) ? WiFiStartupMillis : (cmd == NetworkCommand::networkAddSsid || cmd == NetworkCommand::networkDeleteSsid || cmd == NetworkCommand::networkConfigureAccessPoint || cmd == NetworkCommand::networkRetrieveSsidData diff --git a/src/Networking/ESP8266WiFi/WiFiInterface.h b/src/Networking/ESP8266WiFi/WiFiInterface.h index 74f374f498..02e5b32f61 100644 --- a/src/Networking/ESP8266WiFi/WiFiInterface.h +++ b/src/Networking/ESP8266WiFi/WiFiInterface.h @@ -88,7 +88,7 @@ class WiFiInterface : public NetworkInterface void StartWiFi() noexcept; void ResetWiFi() noexcept; void ResetWiFiForUpload(bool external) noexcept; - const char *GetWiFiServerVersion() const noexcept { return wiFiServerVersion; } + const char *GetWiFiServerVersion() const noexcept { return wiFiServerVersion.c_str(); } static const char* TranslateWiFiState(WiFiState w) noexcept; void SpiInterrupt() noexcept; void EspRequestsTransfer() noexcept; @@ -158,8 +158,8 @@ class WiFiInterface : public NetworkInterface IPAddress netmask; IPAddress gateway; MacAddress macAddress; - char requestedSsid[SsidLength + 1]; - char actualSsid[SsidLength + 1]; + String requestedSsid; + String actualSsid; unsigned int spiTxUnderruns; unsigned int spiRxOverruns; @@ -168,7 +168,7 @@ class WiFiInterface : public NetworkInterface unsigned int readyTimeoutCount = 0; unsigned int responseTimeoutCount = 0; - char wiFiServerVersion[16]; + String wiFiServerVersion; bool usingDhcp = true; uint8_t startupRetryCount;