diff --git a/include/PowerMeterHttpJson.h b/include/PowerMeterHttpJson.h index 9e548210..4b7c8675 100644 --- a/include/PowerMeterHttpJson.h +++ b/include/PowerMeterHttpJson.h @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include #include #include #include @@ -12,6 +13,8 @@ using Unit_t = PowerMeterHttpConfig::Unit; class PowerMeterHttpJson : public PowerMeterProvider { public: + ~PowerMeterHttpJson(); + bool init() final { return true; } void deinit() final { } void loop() final; @@ -25,10 +28,11 @@ private: uint32_t _lastPoll; std::array _cache; std::array _powerValues; - HTTPClient httpClient; + std::unique_ptr wifiClient; + std::unique_ptr httpClient; String httpResponse; - bool httpRequest(int phase, WiFiClient &wifiClient, const String& host, uint16_t port, const String& uri, bool https, PowerMeterHttpConfig const& config); + bool httpRequest(int phase, const String& host, uint16_t port, const String& uri, bool https, PowerMeterHttpConfig const& config); bool extractUrlComponents(String url, String& _protocol, String& _hostname, String& _uri, uint16_t& uint16_t, String& _base64Authorization); String extractParam(String& authReq, const String& param, const char delimit); String getcNonce(const int len); diff --git a/include/PowerMeterHttpSml.h b/include/PowerMeterHttpSml.h index 31b44244..2d100bd3 100644 --- a/include/PowerMeterHttpSml.h +++ b/include/PowerMeterHttpSml.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -12,6 +13,8 @@ class PowerMeterHttpSml : public PowerMeterProvider { public: + ~PowerMeterHttpSml(); + bool init() final { return true; } void deinit() final { } void loop() final; @@ -38,9 +41,10 @@ private: {{0x01, 0x00, 0x10, 0x07, 0x00, 0xff}, &smlOBISW, &_activePower} }; - HTTPClient httpClient; + std::unique_ptr wifiClient; + std::unique_ptr httpClient; String httpResponse; - bool httpRequest(WiFiClient &wifiClient, const String& host, uint16_t port, const String& uri, bool https, PowerMeterTibberConfig const& config); + bool httpRequest(const String& host, uint16_t port, const String& uri, bool https, PowerMeterTibberConfig const& config); bool extractUrlComponents(String url, String& _protocol, String& _hostname, String& _uri, uint16_t& uint16_t, String& _base64Authorization); void prepareRequest(uint32_t timeout); }; diff --git a/src/PowerMeterHttpJson.cpp b/src/PowerMeterHttpJson.cpp index c8d95bd3..ea9d5bee 100644 --- a/src/PowerMeterHttpJson.cpp +++ b/src/PowerMeterHttpJson.cpp @@ -6,9 +6,17 @@ #include #include "mbedtls/sha256.h" #include -#include #include +PowerMeterHttpJson::~PowerMeterHttpJson() +{ + // the wifiClient instance must live longer than the httpClient instance, + // as the httpClient holds a pointer to the wifiClient and uses it in its + // destructor. + httpClient.reset(); + wifiClient.reset(); +} + void PowerMeterHttpJson::loop() { auto const& config = Configuration.get(); @@ -66,7 +74,7 @@ bool PowerMeterHttpJson::queryPhase(int phase, PowerMeterHttpConfig const& confi //hostByName in WiFiGeneric fails to resolve local names. issue described in //https://github.com/espressif/arduino-esp32/issues/3822 //and in depth analyzed in https://github.com/espressif/esp-idf/issues/2507#issuecomment-761836300 - //in conclusion: we cannot rely on httpClient.begin(*wifiClient, url) to resolve IP adresses. + //in conclusion: we cannot rely on httpClient->begin(*wifiClient, url) to resolve IP adresses. //have to do it manually here. Feels Hacky... String protocol; String host; @@ -102,10 +110,6 @@ bool PowerMeterHttpJson::queryPhase(int phase, PowerMeterHttpConfig const& confi } } - // secureWifiClient MUST be created before HTTPClient - // see discussion: https://github.com/helgeerbe/OpenDTU-OnBattery/issues/381 - std::unique_ptr wifiClient; - bool https = protocol == "https"; if (https) { auto secureWifiClient = std::make_unique(); @@ -115,49 +119,51 @@ bool PowerMeterHttpJson::queryPhase(int phase, PowerMeterHttpConfig const& confi wifiClient = std::make_unique(); } - return httpRequest(phase, *wifiClient, ipaddr.toString(), port, uri, https, config); + return httpRequest(phase, ipaddr.toString(), port, uri, https, config); } -bool PowerMeterHttpJson::httpRequest(int phase, WiFiClient &wifiClient, const String& host, uint16_t port, const String& uri, bool https, PowerMeterHttpConfig const& config) +bool PowerMeterHttpJson::httpRequest(int phase, const String& host, uint16_t port, const String& uri, bool https, PowerMeterHttpConfig const& config) { - if(!httpClient.begin(wifiClient, host, port, uri, https)){ - snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("httpClient.begin() failed for %s://%s"), (https ? "https" : "http"), host.c_str()); + if (!httpClient) { httpClient = std::make_unique(); } + + if(!httpClient->begin(*wifiClient, host, port, uri, https)){ + snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("httpClient->begin() failed for %s://%s"), (https ? "https" : "http"), host.c_str()); return false; } prepareRequest(config.Timeout, config.HeaderKey, config.HeaderValue); if (config.AuthType == Auth_t::Digest) { const char *headers[1] = {"WWW-Authenticate"}; - httpClient.collectHeaders(headers, 1); + httpClient->collectHeaders(headers, 1); } else if (config.AuthType == Auth_t::Basic) { String authString = config.Username; authString += ":"; authString += config.Password; String auth = "Basic "; auth.concat(base64::encode(authString)); - httpClient.addHeader("Authorization", auth); + httpClient->addHeader("Authorization", auth); } - int httpCode = httpClient.GET(); + int httpCode = httpClient->GET(); if (httpCode == HTTP_CODE_UNAUTHORIZED && config.AuthType == Auth_t::Digest) { // Handle authentication challenge - if (httpClient.hasHeader("WWW-Authenticate")) { - String authReq = httpClient.header("WWW-Authenticate"); + if (httpClient->hasHeader("WWW-Authenticate")) { + String authReq = httpClient->header("WWW-Authenticate"); String authorization = getDigestAuth(authReq, String(config.Username), String(config.Password), "GET", String(uri), 1); - httpClient.end(); - if(!httpClient.begin(wifiClient, host, port, uri, https)){ - snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("httpClient.begin() failed for %s://%s using digest auth"), (https ? "https" : "http"), host.c_str()); + httpClient->end(); + if(!httpClient->begin(*wifiClient, host, port, uri, https)){ + snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("httpClient->begin() failed for %s://%s using digest auth"), (https ? "https" : "http"), host.c_str()); return false; } prepareRequest(config.Timeout, config.HeaderKey, config.HeaderValue); - httpClient.addHeader("Authorization", authorization); - httpCode = httpClient.GET(); + httpClient->addHeader("Authorization", authorization); + httpCode = httpClient->GET(); } } if (httpCode <= 0) { - snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("HTTP Error %s"), httpClient.errorToString(httpCode).c_str()); + snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("HTTP Error %s"), httpClient->errorToString(httpCode).c_str()); return false; } @@ -166,8 +172,8 @@ bool PowerMeterHttpJson::httpRequest(int phase, WiFiClient &wifiClient, const St return false; } - httpResponse = httpClient.getString(); // very unfortunate that we cannot parse WifiClient stream directly - httpClient.end(); + httpResponse = httpClient->getString(); // very unfortunate that we cannot parse WifiClient stream directly + httpClient->end(); // TODO(schlimmchen): postpone calling tryGetFloatValueForPhase, as it // will be called twice for each phase when doing separate requests. @@ -391,14 +397,14 @@ String PowerMeterHttpJson::sha256(const String& data) { } void PowerMeterHttpJson::prepareRequest(uint32_t timeout, const char* httpHeader, const char* httpValue) { - httpClient.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); - httpClient.setUserAgent("OpenDTU-OnBattery"); - httpClient.setConnectTimeout(timeout); - httpClient.setTimeout(timeout); - httpClient.addHeader("Content-Type", "application/json"); - httpClient.addHeader("Accept", "application/json"); + httpClient->setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); + httpClient->setUserAgent("OpenDTU-OnBattery"); + httpClient->setConnectTimeout(timeout); + httpClient->setTimeout(timeout); + httpClient->addHeader("Content-Type", "application/json"); + httpClient->addHeader("Accept", "application/json"); if (strlen(httpHeader) > 0) { - httpClient.addHeader(httpHeader, httpValue); + httpClient->addHeader(httpHeader, httpValue); } } diff --git a/src/PowerMeterHttpSml.cpp b/src/PowerMeterHttpSml.cpp index f3d8e0c7..a753ed7b 100644 --- a/src/PowerMeterHttpSml.cpp +++ b/src/PowerMeterHttpSml.cpp @@ -16,6 +16,15 @@ void PowerMeterHttpSml::doMqttPublish() const { } +PowerMeterHttpSml::~PowerMeterHttpSml() +{ + // the wifiClient instance must live longer than the httpClient instance, + // as the httpClient holds a pointer to the wifiClient and uses it in its + // destructor. + httpClient.reset(); + wifiClient.reset(); +} + void PowerMeterHttpSml::loop() { auto const& config = Configuration.get(); @@ -38,7 +47,7 @@ bool PowerMeterHttpSml::query(PowerMeterTibberConfig const& config) //hostByName in WiFiGeneric fails to resolve local names. issue described in //https://github.com/espressif/arduino-esp32/issues/3822 //and in depth analyzed in https://github.com/espressif/esp-idf/issues/2507#issuecomment-761836300 - //in conclusion: we cannot rely on httpClient.begin(*wifiClient, url) to resolve IP adresses. + //in conclusion: we cannot rely on httpClient->begin(*wifiClient, url) to resolve IP adresses. //have to do it manually here. Feels Hacky... String protocol; String host; @@ -74,10 +83,6 @@ bool PowerMeterHttpSml::query(PowerMeterTibberConfig const& config) } } - // secureWifiClient MUST be created before HTTPClient - // see discussion: https://github.com/helgeerbe/OpenDTU-OnBattery/issues/381 - std::unique_ptr wifiClient; - bool https = protocol == "https"; if (https) { auto secureWifiClient = std::make_unique(); @@ -87,13 +92,15 @@ bool PowerMeterHttpSml::query(PowerMeterTibberConfig const& config) wifiClient = std::make_unique(); } - return httpRequest(*wifiClient, ipaddr.toString(), port, uri, https, config); + return httpRequest(ipaddr.toString(), port, uri, https, config); } -bool PowerMeterHttpSml::httpRequest(WiFiClient &wifiClient, const String& host, uint16_t port, const String& uri, bool https, PowerMeterTibberConfig const& config) +bool PowerMeterHttpSml::httpRequest(const String& host, uint16_t port, const String& uri, bool https, PowerMeterTibberConfig const& config) { - if(!httpClient.begin(wifiClient, host, port, uri, https)){ - snprintf_P(tibberPowerMeterError, sizeof(tibberPowerMeterError), PSTR("httpClient.begin() failed for %s://%s"), (https ? "https" : "http"), host.c_str()); + if (!httpClient) { httpClient = std::make_unique(); } + + if(!httpClient->begin(*wifiClient, host, port, uri, https)){ + snprintf_P(tibberPowerMeterError, sizeof(tibberPowerMeterError), PSTR("httpClient->begin() failed for %s://%s"), (https ? "https" : "http"), host.c_str()); return false; } @@ -104,12 +111,12 @@ bool PowerMeterHttpSml::httpRequest(WiFiClient &wifiClient, const String& host, authString += config.Password; String auth = "Basic "; auth.concat(base64::encode(authString)); - httpClient.addHeader("Authorization", auth); + httpClient->addHeader("Authorization", auth); - int httpCode = httpClient.GET(); + int httpCode = httpClient->GET(); if (httpCode <= 0) { - snprintf_P(tibberPowerMeterError, sizeof(tibberPowerMeterError), PSTR("HTTP Error %s"), httpClient.errorToString(httpCode).c_str()); + snprintf_P(tibberPowerMeterError, sizeof(tibberPowerMeterError), PSTR("HTTP Error %s"), httpClient->errorToString(httpCode).c_str()); return false; } @@ -118,9 +125,9 @@ bool PowerMeterHttpSml::httpRequest(WiFiClient &wifiClient, const String& host, return false; } - while (httpClient.getStream().available()) { + while (httpClient->getStream().available()) { double readVal = 0; - unsigned char smlCurrentChar = httpClient.getStream().read(); + unsigned char smlCurrentChar = httpClient->getStream().read(); sml_states_t smlCurrentState = smlState(smlCurrentChar); if (smlCurrentState == SML_LISTEND) { for (auto& handler: smlHandlerList) { @@ -133,7 +140,7 @@ bool PowerMeterHttpSml::httpRequest(WiFiClient &wifiClient, const String& host, } } } - httpClient.end(); + httpClient->end(); return true; } @@ -190,10 +197,10 @@ bool PowerMeterHttpSml::extractUrlComponents(String url, String& _protocol, Stri } void PowerMeterHttpSml::prepareRequest(uint32_t timeout) { - httpClient.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); - httpClient.setUserAgent("OpenDTU-OnBattery"); - httpClient.setConnectTimeout(timeout); - httpClient.setTimeout(timeout); - httpClient.addHeader("Content-Type", "application/json"); - httpClient.addHeader("Accept", "application/json"); + httpClient->setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); + httpClient->setUserAgent("OpenDTU-OnBattery"); + httpClient->setConnectTimeout(timeout); + httpClient->setTimeout(timeout); + httpClient->addHeader("Content-Type", "application/json"); + httpClient->addHeader("Accept", "application/json"); }