From 141786ad67c8e78397d6ad4d9f617bee886144d5 Mon Sep 17 00:00:00 2001 From: Kevin Matz Date: Tue, 10 Aug 2021 16:08:09 -0400 Subject: [PATCH] nak and length check brevity --- rdm/device.cpp | 142 +++++++++++++++++++++++----------------------- rdm/message.cpp | 30 ++++++++++ rdm/message.h | 2 + rdm/responder.cpp | 22 +++---- 4 files changed, 111 insertions(+), 85 deletions(-) diff --git a/rdm/device.cpp b/rdm/device.cpp index 0890f3e..c7a99e5 100644 --- a/rdm/device.cpp +++ b/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->appendData(NR_UNKNOWN_PID); + response->nak(NR_UNKNOWN_PID); return false; } @@ -307,9 +306,11 @@ bool Device::actionPrep_(const Message *message, Message *response) * @param message * @param response */ -void Device::actionGetSupportedParameters( - __attribute__((unused)) const Message *message, Message *response) -{ +void Device::actionGetSupportedParameters(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( - __attribute__((unused)) const Message *message, Message *response) +void Device::actionGetDeviceInfo(const Message *message, Message *response) { + if (!message->requiredLength(0, response)) + return; + response->responseType = RESPONSE_TYPE_ACK; response->appendData(RDM_PROTOCOL_VERSION); response->appendData(deviceModelID); @@ -362,9 +365,11 @@ void Device::actionGetDeviceInfo( * @param message * @param response */ -void Device::actionGetProductDetailIdList( - __attribute__((unused)) const Message *message, Message *response) +void Device::actionGetProductDetailIdList(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( - __attribute__((unused)) const Message *message, Message *response) -{ +void Device::actionGetDevModelDescription(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( - __attribute__((unused)) const Message *message, Message *response) +void Device::actionGetManufacturerLabel(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( - __attribute__((unused)) const Message *message, Message *response) +void Device::actionGetLanguage(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) - { - response->responseType = RESPONSE_TYPE_NACK_REASON; - response->appendData(NR_FORMAT_ERROR); - return; - } + if (!message->requiredLength(2, response)) + return; + std::string s; for ( char c : *message->data()) s += c; if (s != "en") { - response->responseType = RESPONSE_TYPE_NACK_REASON; - response->appendData(NR_DATA_OUT_OF_RANGE); + response->nak(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( - __attribute__((unused)) const Message *message, Message *response) +void Device::actionGetSoftwareVersionLabel(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( - __attribute__((unused)) const Message *message, Message *response) +void Device::actionGetDmxPersonality(const Message *message, Message *response) { + if (!message->requiredLength(0, response)) + return; + response->responseType = RESPONSE_TYPE_ACK; response->appendData(DMX::Device::personality()); response->appendData(DMX::Device::personalityCount()); @@ -496,17 +507,14 @@ void Device::actionGetDmxPersonality( */ void Device::actionSetDmxPersonality(const Message *message, Message *response) { - if (message->data()->size() != 1) - { - response->responseType = RESPONSE_TYPE_NACK_REASON; - response->appendData(NR_FORMAT_ERROR); - return; - } + if (!message->requiredLength(1, response)) + return; + uint8_t mode = message->data()->front(); + if ( mode == 0 || mode > DMX::Device::personalityCount()) { - response->responseType = RESPONSE_TYPE_NACK_REASON; - response->appendData(NR_DATA_OUT_OF_RANGE); + response->nak(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) - { - response->responseType = RESPONSE_TYPE_NACK_REASON; - response->appendData(NR_FORMAT_ERROR); - return; - } + if (!message->requiredLength(1, response)) + return; + uint8_t mode = message->data()->front(); + if ( mode == 0 || mode > DMX::Device::personalityCount()) { - response->responseType = RESPONSE_TYPE_NACK_REASON; - response->appendData(NR_DATA_OUT_OF_RANGE); + response->nak(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( - __attribute__((unused)) const Message *message, Message *response) +void Device::actionGetDmxStartAddress(const Message *message, Message *response) { + if (!message->requiredLength(0, response)) + return; + response->responseType = RESPONSE_TYPE_ACK; if (footprint() == 0) response->appendData(0xFFFF); @@ -569,17 +576,14 @@ void Device::actionGetDmxStartAddress( */ void Device::actionSetDmxStartAddress(const Message *message, Message *response) { - if (message->data()->size() != 2) - { - response->responseType = RESPONSE_TYPE_NACK_REASON; - response->appendData(NR_FORMAT_ERROR); - return; - } + if (!message->requiredLength(2, response)) + return; + uint16_t addr = Message::readType(*message->data(), 0); + if (!setAddress(addr)) { - response->responseType = RESPONSE_TYPE_NACK_REASON; - response->appendData(NR_DATA_OUT_OF_RANGE); + response->nak(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) - { - response->responseType = RESPONSE_TYPE_NACK_REASON; - response->appendData(NR_FORMAT_ERROR); - return; - } + if (!message->requiredLength(1, response)) + 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->appendData(NR_DATA_OUT_OF_RANGE); + response->nak(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->appendData(NR_DATA_OUT_OF_RANGE); + response->nak(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( - __attribute__((unused)) const Message *message, Message *response) +void Device::actionGetIdentifyDevice(const Message *message, Message *response) { + if (!message->requiredLength(0, response)) + return; + response->responseType = RESPONSE_TYPE_ACK; response->appendData(identifying_); } @@ -671,12 +672,9 @@ void Device::actionGetIdentifyDevice( */ void Device::actionSetIdentifyDevice(const Message *message, Message *response) { - if (message->data()->size() != 1) - { - response->responseType = RESPONSE_TYPE_NACK_REASON; - response->appendData(NR_FORMAT_ERROR); - return; - } + if (!message->requiredLength(1, response)) + return; + response->responseType = RESPONSE_TYPE_ACK; identify(message->data()->front()); } diff --git a/rdm/message.cpp b/rdm/message.cpp index a99c3e1..90fe319 100644 --- a/rdm/message.cpp +++ b/rdm/message.cpp @@ -224,6 +224,19 @@ void Message::write(std::vector &data) const } +/** + * @brief Message::nak + * @param reason + */ +void Message::nak(uint16_t reason) +{ + data_.clear(); + responseType = RESPONSE_TYPE_NACK_REASON; + appendData(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 diff --git a/rdm/message.h b/rdm/message.h index 9c96d4e..1687afa 100644 --- a/rdm/message.h +++ b/rdm/message.h @@ -39,6 +39,7 @@ struct Message void read(const std::vector &data); void write(std::vector &data) const; + void nak(uint16_t reason); UID source; UID destination; @@ -66,6 +67,7 @@ struct Message const std::vector* 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 void appendData(const T & val) diff --git a/rdm/responder.cpp b/rdm/responder.cpp index f8c1671..0e1bd03 100644 --- a/rdm/responder.cpp +++ b/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) - response->messageCount = 255; + if (queued_messages_.size() > std::numeric_limits::max()) + response->messageCount = std::numeric_limits::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, - __attribute__((unused)) Message* response) +void Responder::rxGet(const Message *message, 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->appendData(NR_SUB_DEVICE_OUT_OF_RANGE); + response->nak(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->appendData(NR_SUB_DEVICE_OUT_OF_RANGE); + response->nak(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, - __attribute__((unused)) Message* response) +void Responder::rxSet(const Message *message, 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->appendData(NR_SUB_DEVICE_OUT_OF_RANGE); + response->nak(NR_SUB_DEVICE_OUT_OF_RANGE); return; }