powermeter refactor: allow destruction of httpClient

make sure the wifiClient used by the httpClient lives longer than the
httpClient, as it accesses the pointer to the wifiClient in its
destructor.

include/PowerMeterHttpJson.h

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #pragma once
 
+#include <memory>
 #include <stdint.h>
 #include <Arduino.h>
 #include <HTTPClient.h>
@@ -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<float,POWERMETER_MAX_PHASES> _cache;
     std::array<float,POWERMETER_MAX_PHASES> _powerValues;
-    HTTPClient httpClient;
+    std::unique_ptr<WiFiClient> wifiClient;
+    std::unique_ptr<HTTPClient> 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);

include/PowerMeterHttpSml.h

@@ -3,6 +3,7 @@
 
 #include <list>
 #include <mutex>
+#include <memory>
 #include <stdint.h>
 #include <Arduino.h>
 #include <HTTPClient.h>
@@ -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> wifiClient;
+    std::unique_ptr<HTTPClient> 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);
 };

src/PowerMeterHttpJson.cpp

@@ -6,9 +6,17 @@
 #include <ArduinoJson.h>
 #include "mbedtls/sha256.h"
 #include <base64.h>
-#include <memory>
 #include <ESPmDNS.h>
 
+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> wifiClient;
-
     bool https = protocol == "https";
     if (https) {
       auto secureWifiClient = std::make_unique<WiFiClientSecure>();
@@ -115,49 +119,51 @@ bool PowerMeterHttpJson::queryPhase(int phase, PowerMeterHttpConfig const& confi
       wifiClient = std::make_unique<WiFiClient>();
     }
 
-    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)){
+    if (!httpClient) { httpClient = std::make_unique<HTTPClient>(); }
-        snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("httpClient.begin() failed for %s://%s"), (https ? "https" : "http"), host.c_str());
+
+    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")) {
+        if (httpClient->hasHeader("WWW-Authenticate")) {
-            String authReq  = httpClient.header("WWW-Authenticate");
+            String authReq  = httpClient->header("WWW-Authenticate");
             String authorization = getDigestAuth(authReq, String(config.Username), String(config.Password), "GET", String(uri), 1);
-            httpClient.end();
+            httpClient->end();
-            if(!httpClient.begin(wifiClient, host, port, uri, https)){
+            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());
+                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);
+            httpClient->addHeader("Authorization", authorization);
-            httpCode = httpClient.GET();
+            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
+    httpResponse = httpClient->getString(); // very unfortunate that we cannot parse WifiClient stream directly
-    httpClient.end();
+    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->setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
-    httpClient.setUserAgent("OpenDTU-OnBattery");
+    httpClient->setUserAgent("OpenDTU-OnBattery");
-    httpClient.setConnectTimeout(timeout);
+    httpClient->setConnectTimeout(timeout);
-    httpClient.setTimeout(timeout);
+    httpClient->setTimeout(timeout);
-    httpClient.addHeader("Content-Type", "application/json");
+    httpClient->addHeader("Content-Type", "application/json");
-    httpClient.addHeader("Accept", "application/json");
+    httpClient->addHeader("Accept", "application/json");
 
     if (strlen(httpHeader) > 0) {
-        httpClient.addHeader(httpHeader, httpValue);
+        httpClient->addHeader(httpHeader, httpValue);
     }
 }

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> wifiClient;
-
     bool https = protocol == "https";
     if (https) {
       auto secureWifiClient = std::make_unique<WiFiClientSecure>();
@@ -87,13 +92,15 @@ bool PowerMeterHttpSml::query(PowerMeterTibberConfig const& config)
       wifiClient = std::make_unique<WiFiClient>();
     }
 
-    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)){
+    if (!httpClient) { httpClient = std::make_unique<HTTPClient>(); }
-        snprintf_P(tibberPowerMeterError, sizeof(tibberPowerMeterError), PSTR("httpClient.begin() failed for %s://%s"), (https ? "https" : "http"), host.c_str());
+
+    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->setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
-    httpClient.setUserAgent("OpenDTU-OnBattery");
+    httpClient->setUserAgent("OpenDTU-OnBattery");
-    httpClient.setConnectTimeout(timeout);
+    httpClient->setConnectTimeout(timeout);
-    httpClient.setTimeout(timeout);
+    httpClient->setTimeout(timeout);
-    httpClient.addHeader("Content-Type", "application/json");
+    httpClient->addHeader("Content-Type", "application/json");
-    httpClient.addHeader("Accept", "application/json");
+    httpClient->addHeader("Accept", "application/json");
 }