nak and length check brevity

rdm/device.cpp

@@ -287,8 +287,7 @@ bool Device::actionPrep_(const Message *message, Message *response)
 {
   if (!parameters_.count(message->propertyID))
     {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
+      response->nak(NR_UNKNOWN_PID);
-      response->appendData<uint16_t>(NR_UNKNOWN_PID);
       return false;
     }
 
@@ -307,9 +306,11 @@ bool Device::actionPrep_(const Message *message, Message *response)
  * @param message
  * @param response
  */
-void Device::actionGetSupportedParameters(
+void Device::actionGetSupportedParameters(const Message *message, Message *response)
-    __attribute__((unused)) const Message *message, Message *response)
 {  
+  if (!message->requiredLength(0, response))
+    return;
+
   uint count = parameters_.size();
   uint length = count * sizeof(PID);
   uint lastPage = length / 0xfe;
@@ -340,9 +341,11 @@ void Device::actionGetSupportedParameters(
  * @param message
  * @param response
  */
-void Device::actionGetDeviceInfo(
+void Device::actionGetDeviceInfo(const Message *message, Message *response)
-    __attribute__((unused)) const Message *message, Message *response)
 {
+  if (!message->requiredLength(0, response))
+    return;
+
   response->responseType = RESPONSE_TYPE_ACK;
   response->appendData<uint16_t>(RDM_PROTOCOL_VERSION);
   response->appendData<uint16_t>(deviceModelID);
@@ -362,9 +365,11 @@ void Device::actionGetDeviceInfo(
  * @param message
  * @param response
  */
-void Device::actionGetProductDetailIdList(
+void Device::actionGetProductDetailIdList(const Message *message, Message *response)
-    __attribute__((unused)) const Message *message, Message *response)
 {
+  if (!message->requiredLength(0, response))
+    return;
+
   response->responseType = RESPONSE_TYPE_ACK;
   if (product_detail_list_.empty())
     {
@@ -381,9 +386,11 @@ void Device::actionGetProductDetailIdList(
  * @param message
  * @param response
  */
-void Device::actionGetDevModelDescription(
+void Device::actionGetDevModelDescription(const Message *message, Message *response)
-    __attribute__((unused)) const Message *message, Message *response)
 {  
+  if (!message->requiredLength(0, response))
+    return;
+
   response->responseType = RESPONSE_TYPE_ACK;
   for (size_t i = 0; i < deviceModelDescription.size(); i++)
     {
@@ -399,9 +406,11 @@ void Device::actionGetDevModelDescription(
  * @param message
  * @param response
  */
-void Device::actionGetManufacturerLabel(
+void Device::actionGetManufacturerLabel(const Message *message, Message *response)
-    __attribute__((unused)) const Message *message, Message *response)
 {
+  if (!message->requiredLength(0, response))
+    return;
+
   response->responseType = RESPONSE_TYPE_ACK;
   std::string label = std::string(MY_ESTA_MANUFACTURER_LABEL);
   for (size_t i = 0; i < label.size(); i++)
@@ -418,9 +427,11 @@ void Device::actionGetManufacturerLabel(
  * @param message
  * @param response
  */
-void Device::actionGetLanguage(
+void Device::actionGetLanguage(const Message *message, Message *response)
-    __attribute__((unused)) const Message *message, Message *response)
 {
+  if (!message->requiredLength(0, response))
+    return;
+
   response->responseType = RESPONSE_TYPE_ACK;
   std::string label = std::string("en");
   for (char& c : label)
@@ -435,20 +446,16 @@ void Device::actionGetLanguage(
  */
 void Device::actionSetLanguage(const Message *message, Message *response)
 {
-  if (message->data()->size() != 2)
+  if (!message->requiredLength(2, response))
-    {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
-      response->appendData<uint16_t>(NR_FORMAT_ERROR);
     return;
-    }
+
   std::string s;
   for ( char c : *message->data())
     s += c;
 
   if (s != "en")
     {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
+      response->nak(NR_DATA_OUT_OF_RANGE);
-      response->appendData<uint16_t>(NR_DATA_OUT_OF_RANGE);
       return;
     }
 
@@ -461,9 +468,11 @@ void Device::actionSetLanguage(const Message *message, Message *response)
  * @param message
  * @param response
  */
-void Device::actionGetSoftwareVersionLabel(
+void Device::actionGetSoftwareVersionLabel(const Message *message, Message *response)
-    __attribute__((unused)) const Message *message, Message *response)
 {
+  if (!message->requiredLength(0, response))
+    return;
+
   response->responseType = RESPONSE_TYPE_ACK;
   std::string label = std::string(LIB_VERSION_LABEL);
   for (size_t i = 0; i < label.size(); i++)
@@ -480,9 +489,11 @@ void Device::actionGetSoftwareVersionLabel(
  * @param message
  * @param response
  */
-void Device::actionGetDmxPersonality(
+void Device::actionGetDmxPersonality(const Message *message, Message *response)
-    __attribute__((unused)) const Message *message, Message *response)
 {
+  if (!message->requiredLength(0, response))
+    return;
+
   response->responseType = RESPONSE_TYPE_ACK;
   response->appendData<uint8_t>(DMX::Device::personality());
   response->appendData<uint8_t>(DMX::Device::personalityCount());
@@ -496,17 +507,14 @@ void Device::actionGetDmxPersonality(
  */
 void Device::actionSetDmxPersonality(const Message *message, Message *response)
 {
-  if (message->data()->size() != 1)
+  if (!message->requiredLength(1, response))
-    {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
-      response->appendData<uint16_t>(NR_FORMAT_ERROR);
     return;
-    }
+
   uint8_t mode = message->data()->front();
+
   if ( mode == 0 || mode > DMX::Device::personalityCount())
     {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
+      response->nak(NR_DATA_OUT_OF_RANGE);
-      response->appendData<uint16_t>(NR_DATA_OUT_OF_RANGE);
       return;
     }
   response->responseType = RESPONSE_TYPE_ACK;
@@ -521,17 +529,14 @@ void Device::actionSetDmxPersonality(const Message *message, Message *response)
  */
 void Device::actionGetDmxPersonalityDesc(const Message *message, Message *response)
 {
-  if (message->data()->size() != 1)
+  if (!message->requiredLength(1, response))
-    {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
-      response->appendData<uint16_t>(NR_FORMAT_ERROR);
     return;
-    }
+
   uint8_t mode = message->data()->front();
+
   if ( mode == 0 || mode > DMX::Device::personalityCount())
     {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
+      response->nak(NR_DATA_OUT_OF_RANGE);
-      response->appendData<uint16_t>(NR_DATA_OUT_OF_RANGE);
       return;
     }
   response->responseType = RESPONSE_TYPE_ACK;
@@ -551,9 +556,11 @@ void Device::actionGetDmxPersonalityDesc(const Message *message, Message *respon
  * @param message
  * @param response
  */
-void Device::actionGetDmxStartAddress(
+void Device::actionGetDmxStartAddress(const Message *message, Message *response)
-    __attribute__((unused)) const Message *message, Message *response)
 {
+  if (!message->requiredLength(0, response))
+    return;
+
   response->responseType = RESPONSE_TYPE_ACK;
   if (footprint() == 0)
     response->appendData<uint16_t>(0xFFFF);
@@ -569,17 +576,14 @@ void Device::actionGetDmxStartAddress(
  */
 void Device::actionSetDmxStartAddress(const Message *message, Message *response)
 {
-  if (message->data()->size() != 2)
+  if (!message->requiredLength(2, response))
-    {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
-      response->appendData<uint16_t>(NR_FORMAT_ERROR);
     return;
-    }
+
   uint16_t addr = Message::readType<uint16_t>(*message->data(), 0);
+
   if (!setAddress(addr))
     {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
+      response->nak(NR_DATA_OUT_OF_RANGE);
-      response->appendData<uint16_t>(NR_DATA_OUT_OF_RANGE);
       return;
     }
   response->responseType = RESPONSE_TYPE_ACK;
@@ -593,12 +597,9 @@ void Device::actionSetDmxStartAddress(const Message *message, Message *response)
  */
 void Device::actionSensorDispatch(const Message *message, Message *response)
 {
-  if (message->data()->size() != 1)
+  if (!message->requiredLength(1, response))
-    {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
-      response->appendData<uint16_t>(NR_FORMAT_ERROR);
     return;
-    }
+
   uint8_t index = message->data()->front();
 
   switch (message->commandClass) {
@@ -606,8 +607,7 @@ void Device::actionSensorDispatch(const Message *message, Message *response)
       {
         if (index == 0xFF || index >= sensors_.size())
           {
-            response->responseType = RESPONSE_TYPE_NACK_REASON;
+            response->nak(NR_DATA_OUT_OF_RANGE);
-            response->appendData<uint16_t>(NR_DATA_OUT_OF_RANGE);
             return;
           }
         auto sensor = sensors_.at(index);
@@ -625,8 +625,7 @@ void Device::actionSensorDispatch(const Message *message, Message *response)
       {
         if (index >= sensors_.size() && index != 0xFF)
           {
-            response->responseType = RESPONSE_TYPE_NACK_REASON;
+            response->nak(NR_DATA_OUT_OF_RANGE);
-            response->appendData<uint16_t>(NR_DATA_OUT_OF_RANGE);
             return;
           }
         auto setSensor = [index, message, response](Sensor * sensor)
@@ -656,9 +655,11 @@ void Device::actionSensorDispatch(const Message *message, Message *response)
  * @param message
  * @param response
  */
-void Device::actionGetIdentifyDevice(
+void Device::actionGetIdentifyDevice(const Message *message, Message *response)
-    __attribute__((unused)) const Message *message, Message *response)
 {
+  if (!message->requiredLength(0, response))
+    return;
+
   response->responseType = RESPONSE_TYPE_ACK;
   response->appendData<uint8_t>(identifying_);
 }
@@ -671,12 +672,9 @@ void Device::actionGetIdentifyDevice(
  */
 void Device::actionSetIdentifyDevice(const Message *message, Message *response)
 {
-  if (message->data()->size() != 1)
+  if (!message->requiredLength(1, response))
-    {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
-      response->appendData<uint16_t>(NR_FORMAT_ERROR);
     return;
-    }
+
   response->responseType = RESPONSE_TYPE_ACK;
   identify(message->data()->front());
 }

rdm/message.cpp

@@ -224,6 +224,19 @@ void Message::write(std::vector<uint8_t> &data) const
 }
 
 
+/**
+ * @brief Message::nak
+ * @param reason
+ */
+void Message::nak(uint16_t reason)
+{
+  data_.clear();
+  responseType = RESPONSE_TYPE_NACK_REASON;
+  appendData<uint16_t>(reason);
+
+}
+
+
 /**
  * @brief Message::checksum
  * @return
@@ -252,4 +265,21 @@ uint16_t Message::checksum() const
   return sum;
 }
 
+
+/**
+ * @brief Message::requiredLength
+ * @param length
+ * @param response
+ * @return
+ */
+bool Message::requiredLength(const size_t length, Message *response) const
+{
+  if (data_.size() != length)
+    {
+      response->nak(NR_FORMAT_ERROR);
+      return false;
+    }
+  return true;
+}
+
 } // namespace RDM

rdm/message.h

@@ -39,6 +39,7 @@ struct Message
 
   void read(const std::vector<uint8_t> &data);
   void write(std::vector<uint8_t> &data) const;
+  void nak(uint16_t reason);
 
   UID source;
   UID destination;
@@ -66,6 +67,7 @@ struct Message
   const std::vector<uint8_t>* data() const { return &data_; }
   uint8_t length() const { return data_.size(); }
   uint16_t checksum() const;
+  bool requiredLength(const size_t length, Message* response) const;
 
   template<typename T>
   void appendData(const T & val)

rdm/responder.cpp

@@ -83,8 +83,8 @@ void Responder::send(Message *response)
   // If a responder has more than 255 messages queued, then the Message Count
   // field shall remain at 255 until the number of queued messages is reduced
   // below that number.
-  if (queued_messages_.size() > 255)
+  if (queued_messages_.size() > std::numeric_limits<uint8_t>::max())
-    response->messageCount = 255;
+    response->messageCount = std::numeric_limits<uint8_t>::max();
   else
     response->messageCount = queued_messages_.size();
 
@@ -170,7 +170,8 @@ void Responder::receive(const Message *message)
       break;
     default:
       delete response;
-      return;
+      response = nullptr;
+      break;
   }
 
   if (response)
@@ -193,8 +194,7 @@ void Responder::rxDiscovery(__attribute__((unused)) const Message *message,
  * @brief Responder::rxGet
  * @param message
  */
-void Responder::rxGet(const Message *message,
+void Responder::rxGet(const Message *message, Message* response)
-                      __attribute__((unused)) Message* response)
 {
   // 9.2.2 Using Sub-Devices
   // Broadcast GET commands sent to the SUB_DEVICE_ALL_CALL Sub-Device ID are
@@ -203,8 +203,7 @@ void Responder::rxGet(const Message *message,
   // NR_SUB_DEVICE_OUT_OF_RANGE.
   if (message->subDevice == SUB_DEVICE_ALL_CALL)
     {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
+      response->nak(NR_SUB_DEVICE_OUT_OF_RANGE);
-      response->appendData<uint16_t>(NR_SUB_DEVICE_OUT_OF_RANGE);
       return;
     }
 
@@ -216,8 +215,7 @@ void Responder::rxGet(const Message *message,
 
   if (!sub_devices_.count(message->subDevice))
     {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
+      response->nak(NR_SUB_DEVICE_OUT_OF_RANGE);
-      response->appendData<uint16_t>(NR_SUB_DEVICE_OUT_OF_RANGE);
       return;
     }
 
@@ -229,8 +227,7 @@ void Responder::rxGet(const Message *message,
  * @brief Responder::rxSet
  * @param message
  */
-void Responder::rxSet(const Message *message,
+void Responder::rxSet(const Message *message, Message* response)
-                      __attribute__((unused)) Message* response)
 {
   if (message->subDevice == 0)
     {
@@ -254,8 +251,7 @@ void Responder::rxSet(const Message *message,
 
   if (!sub_devices_.count(message->subDevice))
     {
-      response->responseType = RESPONSE_TYPE_NACK_REASON;
+      response->nak(NR_SUB_DEVICE_OUT_OF_RANGE);
-      response->appendData<uint16_t>(NR_SUB_DEVICE_OUT_OF_RANGE);
       return;
     }