Add MQTTManager unit test harness and CI

.github/workflows/mqttmanager-tests.yml

@@ -0,0 +1,60 @@
+name: "MQTTManager tests"
+
+on:
+  pull_request:
+    paths:
+      - "docker/MQTTManager/**"
+      - ".github/workflows/mqttmanager-tests.yml"
+  push:
+    branches:
+      - "main"
+      - "devel"
+    paths:
+      - "docker/MQTTManager/**"
+      - ".github/workflows/mqttmanager-tests.yml"
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    # Building the Conan dependencies from source takes about an hour when the cache is cold.
+    timeout-minutes: 120
+    env:
+      # Host directory mounted as the container's Conan package cache (see run_tests.sh).
+      NSPM_TEST_CONAN_CACHE: ${{ github.workspace }}/.conan-cache
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Restore Conan packages
+        id: conan-cache
+        uses: actions/cache/restore@v4
+        with:
+          path: ${{ env.NSPM_TEST_CONAN_CACHE }}
+          key: mqttmanager-conan-${{ hashFiles('docker/MQTTManager/conanfile.py', 'docker/MQTTManager/tests/docker/Dockerfile') }}
+          restore-keys: mqttmanager-conan-
+
+      - name: Build and run tests
+        run: |
+          mkdir -p "$NSPM_TEST_CONAN_CACHE"
+          docker/MQTTManager/run_tests.sh
+        env:
+          NSPM_TEST_RESULTS_DIR: ${{ github.workspace }}/test-results
+
+      # annotate_only writes the job summary and failure annotations without creating a
+      # check run, which needs checks: write and so fails on pull requests from forks.
+      - name: Report test results
+        if: ${{ !cancelled() }}
+        uses: mikepenz/action-junit-report@v6
+        with:
+          report_paths: test-results/*.xml
+          annotate_only: true
+          detailed_summary: true
+          include_passed: true
+          require_tests: true
+
+      # Saved even when the tests fail, so a failing test does not cost another dependency build.
+      - name: Save Conan packages
+        if: always() && steps.conan-cache.outputs.cache-hit != 'true'
+        uses: actions/cache/save@v4
+        with:
+          path: ${{ env.NSPM_TEST_CONAN_CACHE }}
+          key: ${{ steps.conan-cache.outputs.cache-primary-key }}

.gitignore

@@ -41,3 +41,4 @@ firmware/NSPanelManagerFirmware/CMakeLists.txt
 
 # MacOS
 .DS_Store
+docker/MQTTManager/tests/last_run.log

docker/MQTTManager/CMakeLists.txt

@@ -3,13 +3,19 @@ project(nspm_mqttmanager)
 
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
 set(CMAKE_CXX_STANDARD 23)
-set(TEST_MODE 0)
+# Build the unit tests (tests/) and compile the inline TEST_MODE tests into the libraries.
+# Run them with ./run_tests.sh, or `ctest` in the build directory.
+option(NSPM_BUILD_TESTS "Build the MQTTManager unit tests" OFF)
 set(CMAKE_INCLUDE_SRC_DIRECTORY /MQTTManager/include)
 set(PROTOBUF_SRC_DIRECTORY /MQTTManager/include/protobuf)
 
 add_compile_options(-rdynamic -g)
 add_compile_definitions(SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_TRACE BOOST_STACKTRACE_USE_BACKTRACE BOOST_BIND_GLOBAL_PLACEHOLDERS BOOST_STACKTRACE_LIBCXX_RUNTIME_MAY_CAUSE_MEMORY_LEAK)
-add_compile_definitions(TEST_MODE=0)
+if(NSPM_BUILD_TESTS)
+    add_compile_definitions(TEST_MODE=1)
+else()
+    add_compile_definitions(TEST_MODE=0)
+endif()
 
 # Statically link libraries
 IF(DEFINED ENV{STRIP})
@@ -151,7 +157,7 @@ add_executable(${PROJECT_NAME} src/main.cpp)
 target_include_directories(${PROJECT_NAME} PUBLIC ${CMAKE_INCLUDE_SRC_DIRECTORY})
 target_link_libraries(${PROJECT_NAME} MQTT_Manager MQTTManager_Config MQTTManager_HomeAssistantManager MQTTManager_OpenhabManager MQTTManager_EntityManager MQTTManager_NextionImageServer MQTTManager_Scene MQTTManager_Room MQTTManager_WebsocketServer MQTTManager_CommandManager gtest::gtest)
 
-if (TEST_MODE==1)
+if(NSPM_BUILD_TESTS)
     enable_testing()
-    gtest_discover_tests(${PROJECT_NAME})
+    add_subdirectory(tests)
 endif()

docker/MQTTManager/compile_mqttmanager.sh

@@ -29,11 +29,9 @@ while true; do
 done
 
 if [ "$TEST_MODE" -eq 1 ]; then
-  sed -i 's/set(TEST_MODE.*/set(TEST_MODE 1)/g' /MQTTManager/CMakeLists.txt
-  sed -i 's/add_compile_definitions(TEST_MODE.*/add_compile_definitions(TEST_MODE=1)/g' /MQTTManager/CMakeLists.txt
+  NSPM_BUILD_TESTS=ON
 else
-  sed -i 's/set(TEST_MODE.*/set(TEST_MODE 0)/g' /MQTTManager/CMakeLists.txt
-  sed -i 's/add_compile_definitions(TEST_MODE.*/add_compile_definitions(TEST_MODE=0)/g' /MQTTManager/CMakeLists.txt
+  NSPM_BUILD_TESTS=OFF
 fi
 
 if [ -z "$TARGETPLATFORM" ]; then
@@ -139,7 +137,7 @@ conan install . --build=missing -pr:b default -pr:h host
 echo "--> Conan install complete."
 cd build
 source $BUILD_TYPE/generators/conanbuild.sh
-cmake .. -DCMAKE_TOOLCHAIN_FILE=$BUILD_TYPE/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DBUILD_SHARED_LIBS=${BUILD_SHARED_LIBS}
+cmake .. -DCMAKE_TOOLCHAIN_FILE=$BUILD_TYPE/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DBUILD_SHARED_LIBS=${BUILD_SHARED_LIBS} -DNSPM_BUILD_TESTS=${NSPM_BUILD_TESTS}
 cmake --build . --config $BUILD_TYPE -j $(nproc)
 if [ -f /MQTTManager/fix_compile_commands_path.sh ]; then
   source /MQTTManager/fix_compile_commands_path.sh

docker/MQTTManager/include/button/nspm_button.cpp

@@ -18,8 +18,8 @@
 NSPMButton::NSPMButton(uint32_t button_id) : ButtonEntity(button_id) {
   // Process Home Assistant specific details. General button data is loaded in the "Button" constructor.
 
-  if (this->_controller != MQTT_MANAGER_ENTITY_CONTROLLER::HOME_ASSISTANT) {
-    SPDLOG_ERROR("HomeAssistantSwitch has not been recognized as controlled by HOME_ASSISTANT. Will stop processing switch.");
+  if (this->_controller != MQTT_MANAGER_ENTITY_CONTROLLER::NSPM) {
+    SPDLOG_ERROR("NSPMButton has not been recognized as controlled by NSPM. Will stop processing button.");
     return;
   }
 

docker/MQTTManager/include/command_manager/command_manager.hpp

@@ -21,6 +21,13 @@ class CommandManager {
     CommandManager::_callbacks.disconnect(callback);
   }
 
+#if defined(TEST_MODE) && TEST_MODE == 1
+  // Number of attached command callbacks.
+  static size_t test_callback_count() {
+    return CommandManager::_callbacks.num_slots();
+  }
+#endif
+
 private:
   static inline boost::signals2::signal<void(NSPanelMQTTManagerCommand &command)> _callbacks;
 };

docker/MQTTManager/include/entity_manager/entity_manager.cpp

@@ -121,10 +121,13 @@ void EntityManager::load_rooms() {
   SPDLOG_INFO("Loading {} rooms.", room_ids.size());
 
   // Remove room if it does no longer exist
-  EntityManager::_rooms.erase(std::remove_if(EntityManager::_rooms.begin(), EntityManager::_rooms.end(), [&room_ids](auto room) {
-                                return std::find_if(room_ids.begin(), room_ids.end(), [&room](auto id) { return id == room->get_id(); }) == room_ids.end();
-                              }),
-                              EntityManager::_rooms.end());
+  {
+    std::lock_guard<std::mutex> mutex_guard(EntityManager::_rooms_mutex);
+    EntityManager::_rooms.erase(std::remove_if(EntityManager::_rooms.begin(), EntityManager::_rooms.end(), [&room_ids](auto room) {
+                                  return std::find_if(room_ids.begin(), room_ids.end(), [&room](auto id) { return id == room->get_id(); }) == room_ids.end();
+                                }),
+                                EntityManager::_rooms.end());
+  }
 
   // Cause existing room to reload config or add a new room if it does not exist.
   for (auto &room_id : room_ids) {
@@ -142,6 +145,7 @@ void EntityManager::load_rooms() {
     }
   }
 
+  std::lock_guard<std::mutex> mutex_guard(EntityManager::_rooms_mutex);
   std::sort(EntityManager::_rooms.begin(), EntityManager::_rooms.end(), [](const std::shared_ptr<Room> &a, const std::shared_ptr<Room> &b) {
     return a->get_display_order() < b->get_display_order();
   });
@@ -573,9 +577,16 @@ void EntityManager::update_all_rooms_status() {
     uint16_t num_kelvin_lights_ceiling = 0; // Total number of ceiling lights with color temperature
     uint16_t num_kelvin_lights_table = 0;   // Total number of table lights with color temperature
 
+    // Work on a copy, as load_rooms() may change the list meanwhile.
+    std::vector<std::shared_ptr<Room>> rooms;
+    {
+      std::lock_guard<std::mutex> mutex_guard(EntityManager::_rooms_mutex);
+      rooms = EntityManager::_rooms;
+    }
+
     // Determine if any light is on in any of the rooms
     bool any_light_on = false;
-    for (auto room : EntityManager::_rooms) {
+    for (auto room : rooms) {
       std::vector<std::shared_ptr<Light>> entities = room->get_all_entities_by_type<Light>(MQTT_MANAGER_ENTITY_TYPE::LIGHT);
       for (auto light : entities) {
         if (light->get_state() && light->get_controlled_from_main_page()) {
@@ -585,7 +596,7 @@ void EntityManager::update_all_rooms_status() {
       }
     }
 
-    for (auto room : EntityManager::_rooms) {
+    for (auto room : rooms) {
       for (auto &light : room->get_all_entities_by_type<Light>(MQTT_MANAGER_ENTITY_TYPE::LIGHT)) {
         // Light is not controlled from main page, exclude it from calculations.
         if (!light->get_controlled_from_main_page()) {

docker/MQTTManager/include/entity_manager/entity_manager.hpp

@@ -213,6 +213,13 @@ class EntityManager {
   static std::expected<std::shared_ptr<NSPanel>, EntityError> get_nspanel_by_mac(std::string mac);
   static std::expected<int32_t, EntityError> get_room_id_for_panel_id(uint32_t nspanel_id);
 
+#if defined(TEST_MODE) && TEST_MODE == 1
+  // Handles a panel command as if CommandManager had delivered it, without starting init()'s threads.
+  static void test_process_command(NSPanelMQTTManagerCommand &command) {
+    EntityManager::_command_callback(command);
+  }
+#endif
+
 private:
   static inline std::vector<std::shared_ptr<MqttManagerEntity>> _entities;
   static inline std::mutex _entities_mutex;

docker/MQTTManager/include/home_assistant_manager/home_assistant_manager.cpp

@@ -188,6 +188,12 @@ void HomeAssistantManager::_send_auth() {
 
 void HomeAssistantManager::send_json(nlohmann::json &data) {
   data["id"] = HomeAssistantManager::_next_message_id++;
+#if defined(TEST_MODE) && TEST_MODE == 1
+  {
+    std::lock_guard<std::mutex> lock_guard(HomeAssistantManager::_test_sent_messages_mutex);
+    HomeAssistantManager::_test_sent_messages.push_back(data);
+  }
+#endif
   std::string buffer = data.dump();
   HomeAssistantManager::_send_string(buffer);
 }

docker/MQTTManager/include/home_assistant_manager/home_assistant_manager.hpp

@@ -10,6 +10,12 @@
 #include <nlohmann/json_fwd.hpp>
 #include <string>
 
+#if defined(TEST_MODE) && TEST_MODE == 1
+#include <nlohmann/json.hpp>
+#include <utility>
+#include <vector>
+#endif
+
 class HomeAssistantManager {
 public:
   static void init(); // Start a new thread and connect to HA.
@@ -41,7 +47,25 @@ class HomeAssistantManager {
     HomeAssistantManager::_home_assistant_observers[item].disconnect(callback);
   }
 
+#if defined(TEST_MODE) && TEST_MODE == 1
+  // Everything passed to send_json since the last call, whether or not Home Assistant is connected.
+  static std::vector<nlohmann::json> test_take_sent_messages() {
+    std::lock_guard<std::mutex> lock_guard(HomeAssistantManager::_test_sent_messages_mutex);
+    return std::exchange(HomeAssistantManager::_test_sent_messages, {});
+  }
+
+  // Deliver an event as if it had been received from Home Assistant.
+  static void test_process_event(nlohmann::json &event_data) {
+    HomeAssistantManager::_process_home_assistant_event(event_data);
+  }
+#endif
+
 private:
+#if defined(TEST_MODE) && TEST_MODE == 1
+  static inline std::mutex _test_sent_messages_mutex;
+  static inline std::vector<nlohmann::json> _test_sent_messages;
+#endif
+
   // Request all states that Home Assistant has as there is no way to only request state of one object.
   static void _request_all_states();
 

docker/MQTTManager/include/light/openhab_light.cpp

@@ -35,6 +35,12 @@ OpenhabLight::OpenhabLight(uint32_t light_id) : Light(light_id) {
   this->_last_on_off_change = 0;
   this->_last_brightness_change = 0;
   this->_last_color_temp_change = 0;
+  this->_last_group_brightness_item_state_changed_event_ms = 0;
+  this->_last_group_color_temp_item_state_changed_event_ms = 0;
+  this->_last_group_rgb_item_state_changed_event_ms = 0;
+  this->_openhab_group_brightness_item_state_changed_event_thread_running = false;
+  this->_openhab_group_color_temp_item_state_changed_event_thread_running = false;
+  this->_openhab_group_rgb_item_state_changed_event_thread_running = false;
   this->_current_mode = MQTT_MANAGER_LIGHT_MODE::DEFAULT;
   this->_current_brightness = 0;
   this->_current_color_temperature = 0;

docker/MQTTManager/include/mqtt_manager/mqtt_manager.cpp

@@ -218,7 +218,13 @@ void MQTT_Manager::_reconnect_mqtt_client() {
                         "online", true);
 
   // Loop over retained MQTT messages and send them again to the MQTT broker as it may have restarted and lost retained messages.
-  for (auto it = MQTT_Manager::_mqtt_retain_buffer.cbegin(); it != MQTT_Manager::_mqtt_retain_buffer.cend(); it++) {
+  // Publish from a copy, as publish() writes to the buffer.
+  std::unordered_map<std::string, std::string> retained_messages;
+  {
+    std::lock_guard<std::mutex> retain_buffer_guard(MQTT_Manager::_mqtt_retain_buffer_mutex);
+    retained_messages = MQTT_Manager::_mqtt_retain_buffer;
+  }
+  for (auto it = retained_messages.cbegin(); it != retained_messages.cend(); it++) {
     MQTT_Manager::publish(it->first, it->second, true);
   }
 
@@ -250,6 +256,7 @@ void MQTT_Manager::_reconnect_mqtt_client() {
 
   // Send buffered messages if any
   try {
+    std::lock_guard<std::mutex> mutex_guard(MQTT_Manager::_mqtt_client_mutex);
     auto it = MQTT_Manager::_mqtt_messages_buffer.cbegin();
     while (it != MQTT_Manager::_mqtt_messages_buffer.cend()) {
       MQTT_Manager::_mqtt_client->publish((*it));
@@ -292,7 +299,15 @@ void MQTT_Manager::publish(const std::string &topic, const std::string &payload,
     return; // It's not allowed to publish to empty topic.
   }
 
+#if defined(TEST_MODE) && TEST_MODE == 1
+  {
+    std::lock_guard<std::mutex> lock_guard(MQTT_Manager::_test_published_messages_mutex);
+    MQTT_Manager::_test_published_messages.push_back({topic, payload, retain});
+  }
+#endif
+
   if (retain) {
+    std::lock_guard<std::mutex> retain_buffer_guard(MQTT_Manager::_mqtt_retain_buffer_mutex);
     MQTT_Manager::_mqtt_retain_buffer[topic] = payload;
   }
 
@@ -346,7 +361,10 @@ void MQTT_Manager::clear_retain(const std::string &topic) {
     return;
   }
 
-  MQTT_Manager::_mqtt_retain_buffer.erase(topic);
+  {
+    std::lock_guard<std::mutex> retain_buffer_guard(MQTT_Manager::_mqtt_retain_buffer_mutex);
+    MQTT_Manager::_mqtt_retain_buffer.erase(topic);
+  }
 
   std::lock_guard<std::mutex> mutex_guard(MQTT_Manager::_mqtt_client_mutex);
   WebsocketServer::set_stomp_topic_retained(fmt::format("mqtt/{}", topic), false);

docker/MQTTManager/include/mqtt_manager/mqtt_manager.hpp

@@ -17,6 +17,11 @@
 #include <string>
 #include <unordered_map>
 
+#if defined(TEST_MODE) && TEST_MODE == 1
+#include <utility>
+#include <vector>
+#endif
+
 struct MQTTMessage {
   std::string topic;
   std::string message;
@@ -106,14 +111,39 @@ class MQTT_Manager {
    */
   static void clear_retain(const std::string &topic);
 
+#if defined(TEST_MODE) && TEST_MODE == 1
+  struct TestPublishedMessage {
+    std::string topic;
+    std::string payload;
+    bool retain;
+  };
+
+  // Everything passed to publish since the last call, whether or not the MQTT client is connected.
+  static std::vector<TestPublishedMessage> test_take_published_messages() {
+    std::lock_guard<std::mutex> lock_guard(MQTT_Manager::_test_published_messages_mutex);
+    return std::exchange(MQTT_Manager::_test_published_messages, {});
+  }
+
+  // Number of callbacks attached to a topic.
+  static size_t test_callback_count(const std::string &topic) {
+    std::lock_guard<std::mutex> mutex_guard(MQTT_Manager::_mqtt_client_mutex);
+    return MQTT_Manager::_mqtt_callbacks.count(topic) > 0 ? MQTT_Manager::_mqtt_callbacks.at(topic).num_slots() : 0;
+  }
+#endif
+
 private:
+#if defined(TEST_MODE) && TEST_MODE == 1
+  static inline std::mutex _test_published_messages_mutex;
+  static inline std::vector<TestPublishedMessage> _test_published_messages;
+#endif
   static inline boost::lockfree::spsc_queue<MQTTMessage, boost::lockfree::capacity<256>> _mqtt_message_queue;
   static inline std::thread _process_messages_thread;
   static inline mqtt::client *_mqtt_client = nullptr;
   static inline std::mutex _mqtt_client_mutex;
   static inline std::mutex _mqtt_message_mutex;
   static inline std::list<mqtt::message_ptr> _mqtt_messages_buffer;
   static inline std::unordered_map<std::string, std::string> _mqtt_retain_buffer; // Used so that when we reconnect to an MQTT server that has restarted we can repopulate the retained messages.
+  static inline std::mutex _mqtt_retain_buffer_mutex;
 
   static void _reconnect_mqtt_client();
 

docker/MQTTManager/include/mqtt_manager_config/mqtt_manager_config.hpp

@@ -49,7 +49,6 @@ enum MQTT_MANAGER_SETTING {
   OPENHAB_TOKEN,
   OPENHAB_BRIGHTNESS_CHANNEL_MAX,
   OPENHAB_BRIGHTNESS_CHANNEL_MIN,
-  OPENHAB_RGB_CHANNEL_NAME,
   OUTSIDE_TEMP_SENSOR_ENTITY_ID,
   OUTSIDE_TEMP_SENSOR_PROVIDER,
   WEATHER_PRECIPITATION_FORMAT,