Address @tyeth PR review

Author: brentru

src/Wippersnapper_V2.h

@@ -76,20 +76,12 @@
 #include <string>
 #include <vector>
 
-// Nanopb dependencies
+// Nanopb messages and dependencies
+#include "protos/signal.pb.h"
 #include <nanopb/pb_common.h>
 #include <nanopb/pb_decode.h>
 #include <nanopb/pb_encode.h>
 #include <nanopb/ws_pb_helpers.h>
-#include <pb.h>
-
-// Include Signal Proto
-#include "protos/checkin.pb.h"
-#include "protos/digitalio.pb.h"
-#include "protos/ds18x20.pb.h"
-#include "protos/pixels.pb.h"
-#include "protos/pwm.pb.h"
-#include "protos/signal.pb.h"
 
 // External libraries
 #include "Adafruit_MQTT.h"      // MQTT Client

src/components/pwm/hardware.cpp

@@ -162,9 +162,9 @@ bool PWMHardware::analogWrite(uint32_t value) {
     return false;
   }
 
-  // Calculate duty cycle for the `value` passed in
-  // (assumes 12-bit resolution, 2^12)
-  uint32_t dutyCycle = (uint32_t)((4095.0 / 255.0) * min(value, (uint32_t)255));
+  // Calculate duty cycle for the `value`
+  uint32_t dutyCycle =
+      (uint32_t)(((double)_resolution / 255.0) * min(value, (uint32_t)255));
   return ledcWrite(_pin, dutyCycle);
 }
 

src/components/servo/controller.cpp

@@ -44,6 +44,28 @@ ServoController::~ServoController() {
   }
 }
 
+/**************************************************************************/
+/*!
+    @brief  Publishes a ServoAdded message to the broker
+    @param  servo_pin
+            Pin number of the servo
+    @param  did_attach
+            True if the servo was attached successfully, False otherwise
+    @returns True if successful, False otherwise
+*/
+/**************************************************************************/
+bool ServoController::PublishServoAddedMsg(
+    const char *servo_pin, bool did_attach,
+    wippersnapper_servo_ServoAdd *msg_add) {
+  _servo_model->EncodeServoAdded(msg_add->servo_pin, did_attach);
+  if (!WsV2.PublishSignal(wippersnapper_signal_DeviceToBroker_servo_added_tag,
+                          _servo_model->GetServoAddedMsg())) {
+    WS_DEBUG_PRINTLN("[servo] Error: Failed publishing a ServoAdded message!");
+    return false;
+  }
+  return true;
+}
+
 /**************************************************************************/
 /*!
     @brief  Handles a ServoAdd message
@@ -53,13 +75,16 @@ ServoController::~ServoController() {
 */
 /**************************************************************************/
 bool ServoController::Handle_Servo_Add(pb_istream_t *stream) {
+  bool did_attach;
   if (_active_servo_pins >= MAX_SERVOS) {
     WS_DEBUG_PRINTLN("[servo] Error: Maximum number of servos reached!");
+    PublishServoAddedMsg("PIN_UNKNOWN", false, _servo_model->GetServoAddMsg());
     return false;
   }
 
   if (!_servo_model->DecodeServoAdd(stream)) {
     WS_DEBUG_PRINTLN("[servo] Error: Failed to decode ServoAdd message!");
+    PublishServoAddedMsg("PIN_UNKNOWN", false, _servo_model->GetServoAddMsg());
     return false;
   }
 
@@ -69,7 +94,6 @@ bool ServoController::Handle_Servo_Add(pb_istream_t *stream) {
       pin, (int)msg_add->min_pulse_width, (int)msg_add->max_pulse_width,
       (int)msg_add->servo_freq);
   // Attempt to attach the servo to the pin
-  bool did_attach = false;
   did_attach = _servo_hardware[_active_servo_pins]->ServoAttach();
 
   // Write the pulse width to the servo
@@ -87,12 +111,8 @@ bool ServoController::Handle_Servo_Add(pb_istream_t *stream) {
   }
 
   // Publish ServoAdded message to IO
-  _servo_model->EncodeServoAdded(msg_add->servo_pin, did_attach);
-  if (!WsV2.PublishSignal(wippersnapper_signal_DeviceToBroker_servo_added_tag,
-                          _servo_model->GetServoAddedMsg())) {
-    WS_DEBUG_PRINTLN("[servo] Error: Failed publishing a ServoAdded message!");
+  if (!PublishServoAddedMsg("", false, _servo_model->GetServoAddMsg()))
     return false;
-  }
   return true;
 }
 

src/components/servo/controller.h

@@ -47,6 +47,8 @@ class ServoController {
 
 private:
   int GetServoIndex(uint8_t pin);
+  bool PublishServoAddedMsg(const char *servo_pin, bool did_attach,
+                            wippersnapper_servo_ServoAdd *msg_add);
   ServoModel *_servo_model;
   ServoHardware *_servo_hardware[MAX_SERVOS] = {nullptr};
   int _active_servo_pins; ///< Number of active servo pins