Skip to content

Commit

Permalink
Merge pull request #1535 from peternewman/plugfest-rdm
Browse files Browse the repository at this point in the history 
Various improvements including fixing range validation
  • Loading branch information
@peternewman
peternewman committed Apr 30, 2019
2 parents f7266fa + 2a78b3d commit 997c9fd
Show file tree
Hide file tree
Showing 16 changed files with 314 additions and 26 deletions.
10 changes: 5 additions & 5 deletions common/rdm/CommandPrinter.cpp
View file
Expand Up @@ -209,7 +209,7 @@ void CommandPrinter::DisplayDiscoveryRequest(
UID upper(param_data + UID::UID_SIZE);
*m_output << ", (" << lower << ", " << upper << ")";
} else {
*m_output << ", pdl: " << std::dec << request->ParamDataSize();
*m_output << ", PDL: " << std::dec << request->ParamDataSize();
}
*m_output << endl;
} else {
Expand Down Expand Up @@ -282,7 +282,7 @@ void CommandPrinter::DisplayDiscoveryResponse(
UID upper(param_data + UID::UID_SIZE);
*m_output << ", (" << lower << ", " << upper << ")";
} else {
*m_output << ", pdl: " << response->ParamDataSize();
*m_output << ", PDL: " << response->ParamDataSize();
}
*m_output << endl;
} else {
Expand Down Expand Up @@ -314,8 +314,8 @@ void CommandPrinter::AppendUIDsAndType(const class RDMCommand *command,
const char *message_type) {
*m_output <<
command->SourceUID() << " -> " << command->DestinationUID() << " " <<
message_type << ", sub-device: " << std::dec << command->SubDevice() <<
", tn: " << static_cast<int>(command->TransactionNumber());
message_type << ", Sub-Device: " << std::dec << command->SubDevice() <<
", TN: " << static_cast<int>(command->TransactionNumber());
}


Expand Down Expand Up @@ -393,7 +393,7 @@ void CommandPrinter::AppendPidString(const RDMCommand *command,
if (descriptor) {
*m_output << " (" << descriptor->Name() << ")";
}
*m_output << ", pdl: " << command->ParamDataSize() << endl;
*m_output << ", PDL: " << command->ParamDataSize() << endl;
}


Expand Down
5 changes: 3 additions & 2 deletions common/rdm/RDMCommandTest.cpp
View file
Expand Up @@ -33,6 +33,7 @@
#include "ola/rdm/RDMCommand.h"
#include "ola/rdm/RDMCommandSerializer.h"
#include "ola/rdm/RDMEnums.h"
#include "ola/rdm/RDMPacket.h"
#include "ola/rdm/UID.h"
#include "ola/util/Utils.h"
#include "ola/testing/TestUtils.h"
Expand All @@ -58,7 +59,7 @@ using std::string;
* Calculate a checksum for a packet and update it
*/
void UpdateChecksum(uint8_t *expected, unsigned int expected_length) {
unsigned int checksum = RDMCommand::START_CODE;
unsigned int checksum = ola::rdm::START_CODE;
for (unsigned int i = 0 ; i < expected_length - 2; i++) {
checksum += expected[i];
}
Expand All @@ -68,7 +69,7 @@ void UpdateChecksum(uint8_t *expected, unsigned int expected_length) {
}

void UpdateChecksum(ByteString *data) {
unsigned int checksum = RDMCommand::START_CODE;
unsigned int checksum = ola::rdm::START_CODE;
for (unsigned int i = 0 ; i < data->size() - 2; i++) {
checksum += (*data)[i];
}
Expand Down
8 changes: 6 additions & 2 deletions common/rdm/StringMessageBuilder.cpp
View file
Expand Up @@ -343,8 +343,12 @@ void StringMessageBuilder::VisitInt(
if (descriptor->LookupLabel(input, &int_value) ||
ola::PrefixedHexStringToInt(input, &int_value) ||
ola::StringToInt(input, &int_value)) {
m_groups.top().push_back(
new ola::messaging::BasicMessageField<type>(descriptor, int_value));
if (descriptor->IsValid(int_value)) {
m_groups.top().push_back(
new ola::messaging::BasicMessageField<type>(descriptor, int_value));
} else {
SetError(descriptor->Name());
}
} else {
SetError(descriptor->Name());
}
Expand Down
139 changes: 138 additions & 1 deletion common/rdm/StringMessageBuilderTest.cpp
View file
Expand Up @@ -57,6 +57,8 @@ class StringBuilderTest: public CppUnit::TestFixture {
CPPUNIT_TEST_SUITE(StringBuilderTest);
CPPUNIT_TEST(testSimpleBuilder);
CPPUNIT_TEST(testBuilderWithLabels);
CPPUNIT_TEST(testBuilderWithIntervals);
CPPUNIT_TEST(testBuilderWithLabelsAndIntervals);
CPPUNIT_TEST(testBuilderWithGroups);
CPPUNIT_TEST(testBuilderWithNestedGroups);
CPPUNIT_TEST(testBuilderWithVariableNestedGroups);
Expand All @@ -69,6 +71,8 @@ class StringBuilderTest: public CppUnit::TestFixture {
public:
void testSimpleBuilder();
void testBuilderWithLabels();
void testBuilderWithIntervals();
void testBuilderWithLabelsAndIntervals();
void testBuilderWithGroups();
void testBuilderWithNestedGroups();
void testBuilderWithVariableNestedGroups();
Expand All @@ -82,6 +86,8 @@ class StringBuilderTest: public CppUnit::TestFixture {

const Message *BuildMessage(const Descriptor &descriptor,
const vector<string> &inputs);
const Message *BuildMessageSingleInput(const Descriptor &descriptor,
const string &input);
};


Expand All @@ -97,12 +103,26 @@ const Message *StringBuilderTest::BuildMessage(
const vector<string> &inputs) {
StringMessageBuilder builder;
const Message *message = builder.GetMessage(inputs, &descriptor);
if (!message)
if (!message) {
OLA_WARN << "Error with field: " << builder.GetError();
}
return message;
}


/**
* Build a message from a single input and return the string representation
* of the message.
*/
const Message *StringBuilderTest::BuildMessageSingleInput(
const Descriptor &descriptor,
const string &input) {
vector<string> inputs;
inputs.push_back(input);
return BuildMessage(descriptor, inputs);
}


/**
* Check the StringBuilder works.
*/
Expand Down Expand Up @@ -188,6 +208,123 @@ void StringBuilderTest::testBuilderWithLabels() {

string expected = "uint8: dozen\n";
OLA_ASSERT_EQ(expected, m_printer.AsString(message.get()));

// Test an invalid case
// setup the inputs
vector<string> inputs2;
inputs2.push_back("half_dozen");
auto_ptr<const Message> message2(BuildMessage(descriptor, inputs2));

// verify
OLA_ASSERT_NULL(message2.get());
}


/**
* Check the builder accepts intervals
*/
void StringBuilderTest::testBuilderWithIntervals() {
// build the descriptor
UInt8FieldDescriptor::IntervalVector intervals;
intervals.push_back(UInt16FieldDescriptor::Interval(2, 8));
intervals.push_back(UInt16FieldDescriptor::Interval(12, 14));
UInt8FieldDescriptor::LabeledValues labels;

vector<const FieldDescriptor*> fields;
fields.push_back(new UInt8FieldDescriptor("uint8", intervals, labels));
Descriptor descriptor("Test Descriptor", fields);

auto_ptr<const Message> message(BuildMessageSingleInput(descriptor, "2"));

// verify
OLA_ASSERT_NOT_NULL(message.get());
OLA_ASSERT_EQ(static_cast<unsigned int>(fields.size()),
message->FieldCount());

string expected = "uint8: 2\n";
OLA_ASSERT_EQ(expected, m_printer.AsString(message.get()));


// Test an invalid case
auto_ptr<const Message> message2(BuildMessageSingleInput(descriptor,
"dozen"));

// verify
OLA_ASSERT_NULL(message2.get());

OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "0"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "1"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "2"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "8"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "9"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "11"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "12"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "13"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "14"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "15"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "255"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "65535"));

// check labels
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "one"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "dozen"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "bakers_dozen"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "twenty"));
}


/**
* Check the builder accepts labels and intervals
*/
void StringBuilderTest::testBuilderWithLabelsAndIntervals() {
// build the descriptor
UInt8FieldDescriptor::IntervalVector intervals;
intervals.push_back(UInt16FieldDescriptor::Interval(2, 8));
intervals.push_back(UInt16FieldDescriptor::Interval(12, 14));
UInt8FieldDescriptor::LabeledValues labels;
labels["dozen"] = 12;
labels["bakers_dozen"] = 13;

vector<const FieldDescriptor*> fields;
fields.push_back(new UInt8FieldDescriptor("uint8", intervals, labels));
Descriptor descriptor("Test Descriptor", fields);

auto_ptr<const Message> message(BuildMessageSingleInput(descriptor, "dozen"));

// verify
OLA_ASSERT_NOT_NULL(message.get());
OLA_ASSERT_EQ(static_cast<unsigned int>(fields.size()),
message->FieldCount());

string expected = "uint8: dozen\n";
OLA_ASSERT_EQ(expected, m_printer.AsString(message.get()));


// Test an invalid case
auto_ptr<const Message> message2(BuildMessageSingleInput(descriptor,
"half_dozen"));

// verify
OLA_ASSERT_NULL(message2.get());

OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "0"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "1"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "2"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "8"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "9"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "11"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "12"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "13"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "14"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "15"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "255"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "65535"));

// check labels
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "one"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "dozen"));
OLA_ASSERT_NOT_NULL(BuildMessageSingleInput(descriptor, "bakers_dozen"));
OLA_ASSERT_NULL(BuildMessageSingleInput(descriptor, "twenty"));
}


Expand Down
126 changes: 126 additions & 0 deletions data/rdm/manufacturer_pids.proto
View file
Expand Up @@ -2495,6 +2495,132 @@ manufacturer {
manufacturer {
manufacturer_id: 17742
manufacturer_name: "ENTTEC Pty Ltd"
pid {
name: "AUTO_MODE"
value: 34559
get_request {
}
get_response {
field {
type: UINT8
name: "program"
range {
min: 0
max: 9
}
label {
value: 0
label: "Disabled"
}
label {
value: 6
label: "1 colour chase, 4 chans"
}
label {
value: 7
label: "2 colour chase, 4 chans"
}
label {
value: 8
label: "1 colour chase, 3 chans FW>=1.2"
}
}
field {
type: UINT8
name: "speed"
range {
min: 0
max: 9
}
label {
value: 0
label: "Fastest"
}
label {
value: 9
label: "Slowest"
}
}
field {
type: UINT8
name: "delay"
range {
min: 0
max: 9
}
label {
value: 0
label: "Shortest"
}
label {
value: 9
label: "Longest"
}
}
}
get_sub_device_range: ROOT_OR_SUBDEVICE
set_request {
field {
type: UINT8
name: "program"
range {
min: 0
max: 9
}
label {
value: 0
label: "Disabled"
}
label {
value: 6
label: "1 colour chase, 4 chans"
}
label {
value: 7
label: "2 colour chase, 4 chans"
}
label {
value: 8
label: "1 colour chase, 3 chans FW>=1.2"
}
}
field {
type: UINT8
name: "speed"
range {
min: 0
max: 9
}
label {
value: 0
label: "Fastest"
}
label {
value: 9
label: "Slowest"
}
}
field {
type: UINT8
name: "delay"
range {
min: 0
max: 9
}
label {
value: 0
label: "Shortest"
}
label {
value: 9
label: "Longest"
}
}
}
set_response {
}
set_sub_device_range: ROOT_OR_ALL_SUBDEVICE
}
pid {
name: "PWM_OUTPUT_FREQUENCY"
value: 32770
Expand Down

0 comments on commit 997c9fd

Please sign in to comment.