more warnings and fixes

include/FabUser.h

@@ -25,9 +25,9 @@ struct FabUser
 
   FabUser() = default;
 
-  FabUser(const card::uid_t uid, std::string_view name, bool authenticated, UserLevel level) : card_uid(uid), holder_name(name), authenticated(authenticated), user_level(level) {}
+  FabUser(const card::uid_t uid, std::string_view name, bool auth, UserLevel level) : card_uid(uid), holder_name(name), authenticated(auth), user_level(level) {}
 
-  FabUser(const uint8_t uid[conf::whitelist::UID_BYTE_LEN], std::string_view name, bool authenticated, UserLevel level) : holder_name(name), authenticated(authenticated), user_level(level)
+  FabUser(const uint8_t uid[conf::whitelist::UID_BYTE_LEN], std::string_view name, bool auth, UserLevel level) : holder_name(name), authenticated(auth), user_level(level)
   {
     this->card_uid = card::from_array(uid);
   }

include/MQTTtypes.h

@@ -16,6 +16,7 @@ namespace ServerMQTT
   {
   public:
     virtual std::string payload() const = 0;
+    virtual ~Query() = default;
   };
 
   class UserQuery : public Query
@@ -91,7 +92,7 @@ namespace ServerMQTT
     std::string holder_name;                                         /* Name of the user from server DB */
     FabUser::UserLevel user_level;                                   /* User priviledges */
     UserResponse(bool rok) : request_ok(rok){};
-    UserResponse(bool rok, UserResult result) : request_ok(rok), result(static_cast<uint8_t>(result)){};
+    UserResponse(bool rok, UserResult res) : request_ok(rok), result(static_cast<uint8_t>(res)){};
     static std::unique_ptr<UserResponse> fromJson(JsonDocument &doc);
     UserResult getResult() const;
   };

platformio.ini

@@ -26,7 +26,7 @@ lib_deps =
   Wire
   adafruit/Adafruit NeoPixel@^1.11.0
 build_unflags = -std=gnu++11
-build_flags = -std=gnu++17 -D PINS_ESP32S3 -Wall -Wextra
+build_flags = -std=gnu++17 -D PINS_ESP32S3 -Wall -Wextra -Wshadow -Wnon-virtual-dtor
 check_tool = clangtidy
 check_flags = 
 	clangtidy: --checks=-*,cert-*,clang-analyzer-*,llvm-* --fix
@@ -49,7 +49,7 @@ lib_deps =
 	Wire
   adafruit/Adafruit NeoPixel@^1.11.0
 build_unflags = -std=gnu++11
-build_flags = -std=gnu++17 -D PINS_ESP32 -Wall -Wextra
+build_flags = -std=gnu++17 -D PINS_ESP32 -Wall -Wextra -Wshadow -Wnon-virtual-dtor
 check_tool = clangtidy
 check_flags = 
 	clangtidy: --checks=-*,cert-*,clang-analyzer-*,llvm-* --fix
@@ -73,7 +73,7 @@ lib_deps =
   terrorsl/sMQTTBroker
   adafruit/Adafruit NeoPixel@^1.11.0
 build_unflags = -std=gnu++11
-build_flags = -std=gnu++17 -D WOKWI_SIMULATION -Wall -Wextra
+build_flags = -std=gnu++17 -D WOKWI_SIMULATION -Wall -Wextra -Wshadow -Wnon-virtual-dtor
 check_tool = clangtidy
 check_flags = 
 	clangtidy: --checks=-*,cert-*,clang-analyzer-*,llvm-* --fix

src/AuthProvider.cpp

@@ -10,7 +10,7 @@ namespace Board
   extern FabServer server;
 } // namespace Board
 
-AuthProvider::AuthProvider(WhiteList whitelist) : whitelist(whitelist) {}
+AuthProvider::AuthProvider(WhiteList list) : whitelist(list) {}
 
 /// @brief Checks if the cache contains the card ID, and uses that if available
 /// @param uid card id

src/BoardLogic.cpp

@@ -228,8 +228,8 @@ bool BoardLogic::authorize(card::uid_t uid)
 
       if (this->longTap("Registra"))
       {
-        auto response = Board::server.registerMaintenance(this->user.card_uid);
-        if (!response->request_ok)
+        auto maint_resp = Board::server.registerMaintenance(this->user.card_uid);
+        if (!maint_resp->request_ok)
         {
           this->beep_failed();
           this->changeStatus(Status::ERROR);

src/FabServer.cpp

@@ -15,8 +15,8 @@ using namespace ServerMQTT;
 /// @param password wifi password
 /// @param server_ip server IP address
 /// @param channel wifi channel (use 0 for auto)
-FabServer::FabServer(std::string_view ssid, std::string_view password, std::string_view server_ip, u_int8_t channel)
-    : wifi_ssid(ssid), wifi_password(password), server_ip(server_ip), online(false), channel(channel)
+FabServer::FabServer(std::string_view ssid, std::string_view password, std::string_view ip, u_int8_t chan)
+    : wifi_ssid(ssid), wifi_password(password), server_ip(ip), online(false), channel(chan)
 {
   std::stringstream ss;
   ss << conf::mqtt::topic << "/" << secrets::machine::machine_id.id;
@@ -39,44 +39,45 @@ bool FabServer::publishWithReply(const Query &query)
   }
   return false;
 }
-bool FabServer::publish(String topic, String payload)
+
+bool FabServer::publish(String mqtt_topic, String mqtt_payload)
 {
-  if (payload.length() > FabServer::MAX_MQTT_LENGTH)
+  if (mqtt_payload.length() > FabServer::MAX_MQTT_LENGTH)
   {
-    Serial.printf("MQTT Client: Message is too long: %s\r\n", payload.c_str());
+    Serial.printf("MQTT Client: Message is too long: %s\r\n", mqtt_payload.c_str());
     return false;
   }
 
   this->answer_pending = true;
-  this->last_query = payload.c_str();
+  this->last_query = mqtt_payload.c_str();
 
   if (conf::debug::ENABLE_LOGS)
-    Serial.printf("MQTT Client: sending message %s on topic %s\r\n", payload.c_str(), topic.c_str());
+    Serial.printf("MQTT Client: sending message %s on topic %s\r\n", mqtt_payload.c_str(), mqtt_topic.c_str());
 
-  return this->client.publish(topic.c_str(), payload.c_str());
+  return this->client.publish(mqtt_topic.c_str(), mqtt_payload.c_str());
 }
+
 /// @brief posts to MQTT server
 /// @param query message to post
 /// @return true if the message was published
 bool FabServer::publish(const Query &query)
 {
+  String s_payload(query.payload().data());
+  String s_topic(this->topic.c_str());
 
-  String payload(query.payload().data());
-  String topic(this->topic.c_str());
-
-  if (payload.length() > FabServer::MAX_MQTT_LENGTH)
+  if (s_payload.length() > FabServer::MAX_MQTT_LENGTH)
   {
-    Serial.printf("MQTT Client: Message is too long: %s\r\n", payload.c_str());
+    Serial.printf("MQTT Client: Message is too long: %s\r\n", s_payload.c_str());
     return false;
   }
 
   this->answer_pending = true;
-  this->last_query = payload.c_str();
+  this->last_query = s_payload.c_str();
 
   if (conf::debug::ENABLE_LOGS)
-    Serial.printf("MQTT Client: sending message %s on topic %s\r\n", payload.c_str(), topic.c_str());
+    Serial.printf("MQTT Client: sending message %s on topic %s\r\n", s_payload.c_str(), s_topic.c_str());
 
-  return this->client.publish(topic, payload);
+  return this->client.publish(s_topic, s_payload);
 }
 
 bool FabServer::loop()
@@ -127,16 +128,16 @@ bool FabServer::isOnline() const
 /// @brief Callback for MQTT messages
 /// @param topic topic the message was received on
 /// @param payload payload of the message
-void FabServer::messageReceived(String &topic, String &payload)
+void FabServer::messageReceived(String &s_topic, String &s_payload)
 {
   if (conf::debug::ENABLE_LOGS)
   {
     std::stringstream ss;
-    ss << "MQTT Client: Received " << topic.c_str() << " -> " << payload.c_str();
+    ss << "MQTT Client: Received " << s_topic.c_str() << " -> " << s_payload.c_str();
     Serial.println(ss.str().c_str());
   }
 
-  this->last_reply = payload.c_str();
+  this->last_reply = s_payload.c_str();
   this->answer_pending = false;
 }
 

src/LCDWrapper.tpp

@@ -7,9 +7,9 @@
 #include "Machine.h"
 
 template <uint8_t _COLS, uint8_t _ROWS>
-LCDWrapper<_COLS, _ROWS>::LCDWrapper(const pins_config::lcd_config &config) : config(config),
-                                                                              lcd(config.rs_pin, config.en_pin, config.d0_pin, config.d1_pin, config.d2_pin, config.d3_pin),
-                                                                              show_connection_status(true), show_power_status(true), forceUpdate(true)
+LCDWrapper<_COLS, _ROWS>::LCDWrapper(const pins_config::lcd_config &conf) : config(conf),
+                                                                            lcd(config.rs_pin, config.en_pin, config.d0_pin, config.d1_pin, config.d2_pin, config.d3_pin),
+                                                                            show_connection_status(true), show_power_status(true), forceUpdate(true)
 {
   buffer.fill({0});
   current.fill({0});
@@ -40,12 +40,12 @@ bool LCDWrapper<_COLS, _ROWS>::begin()
 
   if (conf::debug::ENABLE_LOGS)
   {
-    constexpr auto MAX_LEN = 100;
-    char buffer[MAX_LEN] = {0};
-    if (sprintf(buffer, "Configured LCD %d x %d (d4=%d, d5=%d, d6=%d, d7=%d, en=%d, rs=%d), backlight=%d", _COLS, _ROWS,
-                this->config.d0_pin, this->config.d1_pin, this->config.d2_pin, this->config.d3_pin,
-                this->config.en_pin, this->config.rs_pin, this->config.bl_pin) > 0)
-      Serial.println(buffer);
+    constexpr size_t MAX_LEN = 100;
+    char buf[MAX_LEN] = {0};
+    if (snprintf(buf, MAX_LEN, "Configured LCD %d x %d (d4=%d, d5=%d, d6=%d, d7=%d, en=%d, rs=%d), backlight=%d", _COLS, _ROWS,
+                 this->config.d0_pin, this->config.d1_pin, this->config.d2_pin, this->config.d3_pin,
+                 this->config.en_pin, this->config.rs_pin, this->config.bl_pin) > 0)
+      Serial.println(buf);
   }
 
   return true;
@@ -55,14 +55,14 @@ template <uint8_t _COLS, uint8_t _ROWS>
 std::string LCDWrapper<_COLS, _ROWS>::convertSecondsToHHMMSS(duration<uint16_t> duration) const
 {
   //! since something something does not support to_string we have to resort to ye olde cstring stuff
-  char buffer[9] = {0};
+  char buf[9] = {0};
 
-  snprintf(buffer, sizeof(buffer), "%02lu:%02lu:%02lu",
+  snprintf(buf, sizeof(buf), "%02lu:%02lu:%02lu",
            duration.count() / 3600UL,
            (duration.count() % 3600UL) / 60UL,
            duration.count() % 60UL);
 
-  return {buffer};
+  return {buf};
 }
 
 template <uint8_t _COLS, uint8_t _ROWS>
@@ -160,13 +160,13 @@ bool LCDWrapper<_COLS, _ROWS>::needsUpdate(const BoardInfo &bi) const
 }
 
 template <uint8_t _COLS, uint8_t _ROWS>
-void LCDWrapper<_COLS, _ROWS>::prettyPrint(const std::array<std::array<char, _COLS>, _ROWS> &buffer,
+void LCDWrapper<_COLS, _ROWS>::prettyPrint(const std::array<std::array<char, _COLS>, _ROWS> &buf,
                                            const BoardInfo &bi) const
 {
   std::stringstream ss;
   ss << "/" << std::string(_COLS, '-') << "\\\r\n"; // LCD top
 
-  for (auto &row : buffer)
+  for (auto &row : buf)
   {
     ss << "|";
     for (auto &ch : row)

src/Machine.cpp

@@ -12,9 +12,9 @@ using namespace std::chrono;
 
 /// @brief Creates a new machine
 /// @param user_conf configuration of the machine
-Machine::Machine(const Config user_conf, FabServer &server) : maintenanceNeeded(false), allowed(true),
+Machine::Machine(const Config user_conf, FabServer &serv) : maintenanceNeeded(false), allowed(true),
                                                               config(user_conf),
-                                                              server(server),
+                                                              server(serv),
                                                               active(false),
                                                               power_state(PowerState::UNKNOWN)
 {
@@ -143,7 +143,7 @@ void Machine::power_mqtt(bool value)
   if (conf::debug::ENABLE_LOGS)
     Serial.printf("Power set to %d\r\n", value);
 
-  String topic{secrets::mqtt::machine_topic.data()};
+  String topic{secrets::machine::machine_topic.data()};
   String payload = value ? "on" : "off";
 
   uint8_t retries = 0;