API V2 Review - DS18x20 Component

platformio.ini

@@ -71,8 +71,7 @@ lib_deps =
     adafruit/Adafruit TouchScreen
     adafruit/Adafruit MQTT Library
     bblanchon/ArduinoJson
-    https://github.com/PaulStoffregen/OneWire.git
-    https://github.com/milesburton/Arduino-Temperature-Control-Library.git
+    https://github.com/pstolarz/OneWireNg.git
     https://github.com/Sensirion/arduino-sht.git
     https://github.com/Sensirion/arduino-i2c-scd4x.git
     https://github.com/Sensirion/arduino-i2c-sen5x.git
@@ -180,7 +179,8 @@ extends = common:esp32
 board = adafruit_feather_esp32s3
 build_flags = -DARDUINO_ADAFRUIT_FEATHER_ESP32S3 -DBOARD_HAS_PSRAM
 ;set partition to tinyuf2-partitions-4MB.csv as of idf 5.1
-board_build.partitions = tinyuf2-partitions-4MB.csv
+; board_build.partitions = tinyuf2-partitions-4MB.csv
+board_build.partitions = tinyuf2-partitions-4MB-noota.csv
 extra_scripts = pre:rename_usb_config.py
 
 ; Adafruit Feather ESP32-S3 NO PSRAM

src/Wippersnapper.cpp

@@ -66,9 +66,6 @@ Wippersnapper::Wippersnapper() {
 
   // UART
   WS._uartComponent = new ws_uart();
-
-  // DallasSemi (OneWire)
-  WS._ds18x20Component = new ws_ds18x20();
 };
 
 /**************************************************************************/
@@ -1319,43 +1316,6 @@ void cbPWMMsg(char *data, uint16_t len) {
 bool cbDecodeDs18x20Msg(pb_istream_t *stream, const pb_field_t *field,
                         void **arg) {
   (void)arg; // marking unused parameter to avoid compiler warning
-  if (field->tag ==
-      wippersnapper_signal_v1_Ds18x20Request_req_ds18x20_init_tag) {
-    WS_DEBUG_PRINTLN("[Message Type] Init. DS Sensor");
-    // Attempt to decode contents of DS18x20 message
-    wippersnapper_ds18x20_v1_Ds18x20InitRequest msgDS18xInitReq =
-        wippersnapper_ds18x20_v1_Ds18x20InitRequest_init_zero;
-
-    if (!ws_pb_decode(stream,
-                      wippersnapper_ds18x20_v1_Ds18x20InitRequest_fields,
-                      &msgDS18xInitReq)) {
-      WS_DEBUG_PRINTLN("ERROR: Could not decode "
-                       "wippersnapper_ds18x20_v1_Ds18x20InitRequest");
-      return false; // fail out if we can't decode the request
-    }
-    WS_DEBUG_PRINT("Adding DS18x20 Component...");
-    if (!WS._ds18x20Component->addDS18x20(&msgDS18xInitReq))
-      return false;
-    WS_DEBUG_PRINTLN("Added!");
-  } else if (field->tag ==
-             wippersnapper_signal_v1_Ds18x20Request_req_ds18x20_deinit_tag) {
-    WS_DEBUG_PRINTLN("[Message Type] De-init. DS Sensor");
-    // Attempt to decode contents of message
-    wippersnapper_ds18x20_v1_Ds18x20DeInitRequest msgDS18xDeInitReq =
-        wippersnapper_ds18x20_v1_Ds18x20DeInitRequest_init_zero;
-    if (!ws_pb_decode(stream,
-                      wippersnapper_ds18x20_v1_Ds18x20DeInitRequest_fields,
-                      &msgDS18xDeInitReq)) {
-      WS_DEBUG_PRINTLN("ERROR: Could not decode "
-                       "wippersnapper_ds18x20_v1_Ds18x20DeInitRequest");
-      return false; // fail out if we can't decode the request
-    }
-    // exec. deinit request
-    WS._ds18x20Component->deleteDS18x20(&msgDS18xDeInitReq);
-  } else {
-    WS_DEBUG_PRINTLN("ERROR: DS Message type not found!");
-    return false;
-  }
   return true;
 }
 
@@ -2876,10 +2836,6 @@ ws_status_t Wippersnapper::run() {
     WS._i2cPort0->update();
   WS.feedWDT();
 
-  // Process DS18x20 sensor events
-  WS._ds18x20Component->update();
-  WS.feedWDT();
-
   // Process UART sensor events
   WS._uartComponent->update();
   WS.feedWDT();

src/Wippersnapper.h

@@ -64,7 +64,6 @@
 #include "display/ws_display_ui_helper.h"
 #endif
 
-#include "components/ds18x20/ws_ds18x20.h"
 #include "components/pixels/ws_pixels.h"
 #include "components/pwm/ws_pwm.h"
 #include "components/servo/ws_servo.h"
@@ -104,7 +103,6 @@ class ws_ledc;
 class WipperSnapper_Component_I2C;
 class ws_servo;
 class ws_pwm;
-class ws_ds18x20;
 class ws_pixels;
 class ws_uart;
 
@@ -227,7 +225,6 @@ class Wippersnapper {
   ws_pixels *_ws_pixelsComponent; ///< ptr to instance of ws_pixels class
   ws_pwm *_pwmComponent;          ///< Instance of pwm class
   ws_servo *_servoComponent;      ///< Instance of servo class
-  ws_ds18x20 *_ds18x20Component;  ///< Instance of DS18x20 class
   ws_uart *_uartComponent;        ///< Instance of UART class
 
   // TODO: does this really need to be global?

src/Wippersnapper_V2.cpp

@@ -65,15 +65,13 @@ Wippersnapper_V2::Wippersnapper_V2() {
   // UART
   WsV2._uartComponentV2 = new ws_uart();
 
-  // DallasSemi (OneWire)
-  WsV2._ds18x20ComponentV2 = new ws_ds18x20();
-
   // Initialize model classes
   WsV2.sensorModel = new SensorModel();
 
   // Initialize controller classes
   WsV2.digital_io_controller = new DigitalIOController();
   WsV2.analogio_controller = new AnalogIOController();
+  WsV2._ds18x20_controller = new DS18X20Controller();
 };
 
 /**************************************************************************/
@@ -374,6 +372,20 @@ bool cbDecodeBrokerToDevice(pb_istream_t *stream, const pb_field_t *field,
       return false;
     }
     break;
+  case wippersnapper_signal_BrokerToDevice_ds18x20_add_tag:
+    WS_DEBUG_PRINTLN("-> DS18X20 Add Message Type");
+    if (!WsV2._ds18x20_controller->Handle_Ds18x20Add(stream)) {
+      WS_DEBUG_PRINTLN("ERROR: Unable to add DS18X20 sensor!");
+      return false;
+    }
+    break;
+  case wippersnapper_signal_BrokerToDevice_ds18x20_remove_tag:
+    WS_DEBUG_PRINTLN("-> DS18X20 Remove Message Type");
+    if (!WsV2._ds18x20_controller->Handle_Ds18x20Remove(stream)) {
+      WS_DEBUG_PRINTLN("ERROR: Unable to remove DS18X20 sensor!");
+      return false;
+    }
+    break;
   default:
     WS_DEBUG_PRINTLN("ERROR: BrokerToDevice message type not found!");
     return false;
@@ -857,33 +869,53 @@ void Wippersnapper_V2::haltErrorV2(String error,
 */
 /**************************************************************************/
 bool Wippersnapper_V2::PublishSignal(pb_size_t which_payload, void *payload) {
+
+  #ifdef DEBUG_PROFILE
+  unsigned long total_start_time = micros();
+  #endif
+
   size_t szMessageBuf;
   wippersnapper_signal_DeviceToBroker MsgSignal =
       wippersnapper_signal_DeviceToBroker_init_default;
+
   // Fill generic signal payload with the payload from the args.
   WS_DEBUG_PRINT("Signal Payload Type: ");
   switch (which_payload) {
   case wippersnapper_signal_DeviceToBroker_checkin_request_tag:
-    WS_DEBUG_PRINTLN("Checkin Request");
+    WS_DEBUG_PRINTLN("CheckinRequest");
     MsgSignal.which_payload =
         wippersnapper_signal_DeviceToBroker_checkin_request_tag;
     MsgSignal.payload.checkin_request =
         *(wippersnapper_checkin_CheckinRequest *)payload;
     break;
   case wippersnapper_signal_DeviceToBroker_digitalio_event_tag:
-    WS_DEBUG_PRINTLN("DigitalIO Event");
+    WS_DEBUG_PRINTLN("DigitalIOEvent");
     MsgSignal.which_payload =
         wippersnapper_signal_DeviceToBroker_digitalio_event_tag;
     MsgSignal.payload.digitalio_event =
         *(wippersnapper_digitalio_DigitalIOEvent *)payload;
     break;
   case wippersnapper_signal_DeviceToBroker_analogio_event_tag:
-    WS_DEBUG_PRINTLN("AnalogIO Event");
+    WS_DEBUG_PRINTLN("AnalogIOEvent");
     MsgSignal.which_payload =
         wippersnapper_signal_DeviceToBroker_analogio_event_tag;
     MsgSignal.payload.analogio_event =
         *(wippersnapper_analogio_AnalogIOEvent *)payload;
     break;
+  case wippersnapper_signal_DeviceToBroker_ds18x20_added_tag:
+    WS_DEBUG_PRINTLN("DS18X20Added");
+    MsgSignal.which_payload =
+        wippersnapper_signal_DeviceToBroker_ds18x20_added_tag;
+    MsgSignal.payload.ds18x20_added =
+        *(wippersnapper_ds18x20_Ds18x20Added *)payload;
+    break;
+  case wippersnapper_signal_DeviceToBroker_ds18x20_event_tag:
+    WS_DEBUG_PRINTLN("DS18X20Event");
+    MsgSignal.which_payload =
+        wippersnapper_signal_DeviceToBroker_ds18x20_event_tag;
+    MsgSignal.payload.ds18x20_event =
+        *(wippersnapper_ds18x20_Ds18x20Event *)payload;
+    break;
   default:
     WS_DEBUG_PRINTLN("ERROR: Invalid signal payload type, bailing out!");
     return false;
@@ -910,20 +942,37 @@ bool Wippersnapper_V2::PublishSignal(pb_size_t which_payload, void *payload) {
         "ERROR: Unable to encode d2b signal message, bailing out!");
     return false;
   }
-  WS_DEBUG_PRINTLN("Encoded!")
+  WS_DEBUG_PRINTLN("Encoded!");
 
-  //. Check that we are still connected
+  // Check that we are still connected
   runNetFSMV2();
   WsV2.feedWDTV2();
 
+  #ifdef DEBUG_PROFILE
+  unsigned long publish_start_time = micros();
+  #endif
+
   // Attempt to publish the signal message to the broker
   WS_DEBUG_PRINT("Publishing signal message to broker...");
+  #ifdef DEBUG_PROFILE
+  WS_DEBUG_PRINT("Message buffer size: ");
+  WS_DEBUG_PRINTLN(szMessageBuf);
+  #endif
   if (!WsV2._mqttV2->publish(WsV2._topicD2b, msgBuf, szMessageBuf, 1)) {
     WS_DEBUG_PRINTLN("ERROR: Failed to publish signal message to broker!");
     return false;
   }
   WS_DEBUG_PRINTLN("Published!");
 
+  #ifdef DEBUG_PROFILE
+  unsigned long publish_end_time = micros();
+  WS_DEBUG_PRINT("Publishing time: ");
+  WS_DEBUG_PRINTLN(publish_end_time - publish_start_time);
+  unsigned long total_end_time = micros();
+  WS_DEBUG_PRINT("Total PublishSignal() execution time: ");
+  WS_DEBUG_PRINTLN(total_end_time - total_start_time);
+  #endif
+
   return true;
 }
 
@@ -1167,6 +1216,9 @@ ws_status_t Wippersnapper_V2::runV2() {
   // Process all analog inputs
   WsV2.analogio_controller->update();
 
+  // Process all DS18x20 sensor events
+  WsV2._ds18x20_controller->update();
+
   // TODO: Process I2C sensor events
 
   // TODO: Process DS18x20 sensor events

src/Wippersnapper_V2.h

@@ -32,6 +32,7 @@
 #include "protos/checkin.pb.h"
 #include "protos/digitalio.pb.h"
 #include "protos/signal.pb.h"
+#include "protos/ds18x20.pb.h"
 
 // External libraries
 #include "Adafruit_MQTT.h"      // MQTT Client
@@ -48,15 +49,15 @@
 #endif
 
 // Components (API v2)
-#include "components/checkin/model.h"
 #include "components/sensor/model.h"
+#include "components/checkin/model.h"
 #include "components/digitalIO/controller.h"
 #include "components/analogio/controller.h"
+#include "components/ds18x20/controller.h"
 
 
 // Components (API v1)
 #include "components/analogIO/Wippersnapper_AnalogIO.h"
-#include "components/ds18x20/ws_ds18x20.h"
 #include "components/i2c/WipperSnapper_I2C.h"
 #include "components/pixels/ws_pixels.h"
 #include "components/pwm/ws_pwm.h"
@@ -71,8 +72,6 @@
 #include "display/ws_display_ui_helper.h"
 #endif
 
-
-
 #include "provisioning/ConfigJson.h"
 #if defined(USE_TINYUSB)
 #include "provisioning/tinyusb/Wippersnapper_FS_V2.h"
@@ -81,6 +80,10 @@
 #include "provisioning/littlefs/WipperSnapper_LittleFS.h"
 #endif
 
+// Debug print macros
+// Uncomment the following to enable debug output for function profiling
+// #DEBUG_PROFILE 1
+
 #define WS_VERSION                                                             \
   "2.0.0-alpha.1" ///< WipperSnapper app. version (semver-formatted)
 
@@ -90,9 +93,6 @@
 #define WS_KEEPALIVE_INTERVAL_MS                                               \
   5000 ///< Session keepalive interval time, in milliseconds
 
-#define WS_MQTT_MAX_PAYLOAD_SIZE                                               \
-  512 ///< MAXIMUM expected payload size, in bytes
-
 // Forward declarations (API v1)
 class Wippersnapper_AnalogIO;
 class Wippersnapper_FS_V2;
@@ -107,7 +107,6 @@ class ws_ledc;
 class WipperSnapper_Component_I2C;
 class ws_servo;
 class ws_pwm;
-class ws_ds18x20;
 class ws_pixels;
 class ws_uart;
 
@@ -116,6 +115,7 @@ class CheckinModel;
 class SensorModel;
 class DigitalIOController;
 class AnalogIOController;
+class DS18X20Controller;
 
 /**************************************************************************/
 /*!
@@ -214,7 +214,6 @@ class Wippersnapper_V2 {
   ws_pixels *_ws_pixelsComponentV2; ///< ptr to instance of ws_pixels class
   ws_pwm *_pwmComponentV2;          ///< Instance of pwm class
   ws_servo *_servoComponentV2;      ///< Instance of servo class
-  ws_ds18x20 *_ds18x20ComponentV2;  ///< Instance of DS18x20 class
   ws_uart *_uartComponentV2;        ///< Instance of UART class
 
   // API v2 Components
@@ -223,6 +222,7 @@ class Wippersnapper_V2 {
   DigitalIOController
       *digital_io_controller; ///< Instance of DigitalIO controller class
   AnalogIOController *analogio_controller; ///< Instance of AnalogIO controller
+  DS18X20Controller *_ds18x20_controller; ///< Instance of DS18X20 controller
 
   // TODO: does this really need to be global?
   uint8_t _macAddrV2[6];  /*!< Unique network iface identifier */

src/components/digitalIO/model.cpp

@@ -46,6 +46,8 @@ wippersnapper_digitalio_DigitalIOAdd *DigitalIOModel::GetDigitalIOAddMsg() {
 /***********************************************************************/
 /*!
     @brief  Parses a DigitalIORemove message.
+    @param  stream
+            The nanopb input stream.
     @return DigitalIORemove message object.
 */
 /***********************************************************************/