Skip to content

Commit 98cdfc9

Browse files
mathertelmathertel
authored
Merge pull request #37 from peternewman/productise
Fix some more OLA RDM test suite discovered issues
2 parents 6f07a5d + e1f293e commit 98cdfc9

File tree

2 files changed

+104
-80
lines changed

2 files changed

+104
-80
lines changed

‎src/DMXSerial2.cpp

+99-75
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ struct EEPROMVALUES {
222222
byte sig1; // 0x6D signature 1, EPROM values are valid of both signatures match.
223223
byte sig2; // 0x68 signature 2
224224
uint16_t startAddress; // the DMX start address can be changed by a RDM command.
225-
char deviceLabel[DMXSERIAL_MAX_RDM_STRING_LENGTH]; // the device Label can be changed by a RDM command.
225+
char deviceLabel[DMXSERIAL_MAX_RDM_STRING_LENGTH]; // the device Label can be changed by a RDM command. Don't store the null in the EPROM for backwards compatibility.
226226
DEVICEID deviceID; // store the device ID to allow easy software updates.
227227
}; // struct EEPROMVALUES
228228

@@ -255,7 +255,7 @@ DEVICEID _devIDAll = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
255255

256256

257257
// This is the buffer for RDM packets being received and sent.
258-
// this structure is needed to RDM data separate from DMX data.
258+
// This structure is needed to separate RDM data from DMX data.
259259
union RDMMEM {
260260
// the most common RDM packet layout for commands
261261
struct RDMDATA packet;
@@ -264,10 +264,12 @@ union RDMMEM {
264264
struct DISCOVERYMSG discovery;
265265

266266
// the byte array used while receiving and sending.
267-
byte buffer[60];
267+
// This is the max size of any RDM packet and it's max PDL, to allow an
268+
// aribitrary length packet to be received. Thanks to the union it doesn't
269+
// need any more memory
270+
byte buffer[sizeof(packet)];
268271
} _rdm; // union RDMMEM
269272

270-
271273
// This flag will be set when a full RDM packet was received.
272274
bool8 _rdmAvailable;
273275

@@ -354,7 +356,8 @@ void DMXSerialClass2::init(struct RDMINIT *initData, RDMCallbackFunction func, R
354356
// check if the EEPROM values are from the RDM library
355357
if ((eeprom.sig1 == 0x6D) && (eeprom.sig2 == 0x68)) {
356358
_startAddress = eeprom.startAddress;
357-
strcpy (deviceLabel, eeprom.deviceLabel);
359+
// Restricting this to 32 characters or the length, whichever is shorter
360+
strncpy(deviceLabel, eeprom.deviceLabel, DMXSERIAL_MAX_RDM_STRING_LENGTH);
358361
DeviceIDCpy(_devID, eeprom.deviceID);
359362

360363
// setup the manufacturer addressing device-ID
@@ -364,7 +367,7 @@ void DMXSerialClass2::init(struct RDMINIT *initData, RDMCallbackFunction func, R
364367
} else {
365368
// set default values
366369
_startAddress = 1;
367-
strcpy (deviceLabel, "new");
370+
strncpy(deviceLabel, "new", DMXSERIAL_MAX_RDM_STRING_LENGTH);
368371
_devID[4] = random255(); // random(255);
369372
_devID[5] = random255(); // random(255);
370373
} // if
@@ -500,7 +503,7 @@ void DMXSerialClass2::tick(void)
500503
// check if my _devID is in the discovery range
501504
if ((DeviceIDCmp(rdm->Data, _devID) <= 0) && (DeviceIDCmp(_devID, rdm->Data+6) <= 0)) {
502505

503-
// respond a special discovery message !
506+
// respond with a special discovery message !
504507
struct DISCOVERYMSG *disc = &_rdm.discovery;
505508
_rdmCheckSum = 6 * 0xFF;
506509

@@ -595,7 +598,7 @@ void DMXSerialClass2::term(void)
595598

596599
// Process the RDM Command Message by changing the _rdm buffer and returning (true).
597600
// if returning (false) a NAK will be sent.
598-
// This method processes the commands/parameters regarding mute, DEviceInfo, devicelabel,
601+
// This method processes the commands/parameters regarding mute, DeviceInfo, devicelabel,
599602
// manufacturer label, DMX Start address.
600603
// When parameters are changed by a SET command they are persisted into EEPROM.
601604
// When doRespond is true, send an answer back to the controller node.
@@ -638,59 +641,74 @@ void DMXSerialClass2::_processRDMMessage(byte CmdClass, uint16_t Parameter, bool
638641
}
639642
} // if
640643

641-
} else if ((CmdClass == E120_GET_COMMAND) && (Parameter == SWAPINT(E120_DEVICE_INFO))) { // 0x0060
642-
if (_rdm.packet.DataLength > 0) {
643-
// Unexpected data
644-
nackReason = E120_NR_FORMAT_ERROR;
645-
} else if (_rdm.packet.SubDev != 0) {
646-
// No sub-devices supported
647-
nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
648-
} else {
649-
// return all device info data
650-
DEVICEINFO *devInfo = (DEVICEINFO *)(_rdm.packet.Data); // The data has to be responded in the Data buffer.
651-
652-
devInfo->protocolMajor = 1;
653-
devInfo->protocolMinor = 0;
654-
devInfo->deviceModel = SWAPINT(_initData->deviceModelId);
655-
devInfo->productCategory = SWAPINT(E120_PRODUCT_CATEGORY_DIMMER_CS_LED);
656-
devInfo->softwareVersion = SWAPINT32(0x01000000);// 0x04020900;
657-
devInfo->footprint = SWAPINT(_initData->footprint);
658-
devInfo->currentPersonality = 1;
659-
devInfo->personalityCount = 1;
660-
devInfo->startAddress = SWAPINT(_startAddress);
661-
devInfo->subDeviceCount = 0;
662-
devInfo->sensorCount = _initData->sensorsLength;
663-
664-
_rdm.packet.DataLength = sizeof(DEVICEINFO);
665-
handled = true;
644+
} else if (Parameter == SWAPINT(E120_DEVICE_INFO)) { // 0x0060
645+
if (CmdClass == E120_GET_COMMAND) {
646+
if (_rdm.packet.DataLength > 0) {
647+
// Unexpected data
648+
nackReason = E120_NR_FORMAT_ERROR;
649+
} else if (_rdm.packet.SubDev != 0) {
650+
// No sub-devices supported
651+
nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
652+
} else {
653+
// return all device info data
654+
DEVICEINFO *devInfo = (DEVICEINFO *)(_rdm.packet.Data); // The data has to be responded in the Data buffer.
655+
656+
devInfo->protocolMajor = 1;
657+
devInfo->protocolMinor = 0;
658+
devInfo->deviceModel = SWAPINT(_initData->deviceModelId);
659+
devInfo->productCategory = SWAPINT(E120_PRODUCT_CATEGORY_DIMMER_CS_LED);
660+
devInfo->softwareVersion = SWAPINT32(0x01000000);// 0x04020900;
661+
devInfo->footprint = SWAPINT(_initData->footprint);
662+
devInfo->currentPersonality = 1;
663+
devInfo->personalityCount = 1;
664+
devInfo->startAddress = SWAPINT(_startAddress);
665+
devInfo->subDeviceCount = 0;
666+
devInfo->sensorCount = _initData->sensorsLength;
667+
668+
_rdm.packet.DataLength = sizeof(DEVICEINFO);
669+
handled = true;
670+
}
671+
} else if (CmdClass == E120_SET_COMMAND) {
672+
// Unexpected set
673+
nackReason = E120_NR_UNSUPPORTED_COMMAND_CLASS;
666674
}
667675

668-
} else if ((CmdClass == E120_GET_COMMAND) && (Parameter == SWAPINT(E120_MANUFACTURER_LABEL))) { // 0x0081
669-
if (_rdm.packet.DataLength > 0) {
670-
// Unexpected data
671-
nackReason = E120_NR_FORMAT_ERROR;
672-
} else if (_rdm.packet.SubDev != 0) {
673-
// No sub-devices supported
674-
nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
675-
} else {
676-
// return the manufacturer label
677-
_rdm.packet.DataLength = strlen(_initData->manufacturerLabel);
678-
memcpy(_rdm.packet.Data, _initData->manufacturerLabel, _rdm.packet.DataLength);
679-
handled = true;
676+
} else if (Parameter == SWAPINT(E120_MANUFACTURER_LABEL)) { // 0x0081
677+
if (CmdClass == E120_GET_COMMAND) {
678+
if (_rdm.packet.DataLength > 0) {
679+
// Unexpected data
680+
nackReason = E120_NR_FORMAT_ERROR;
681+
} else if (_rdm.packet.SubDev != 0) {
682+
// No sub-devices supported
683+
nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
684+
} else {
685+
// return the manufacturer label
686+
_rdm.packet.DataLength = strnlen(_initData->manufacturerLabel, DMXSERIAL_MAX_RDM_STRING_LENGTH);
687+
memcpy(_rdm.packet.Data, _initData->manufacturerLabel, _rdm.packet.DataLength);
688+
handled = true;
689+
}
690+
} else if (CmdClass == E120_SET_COMMAND) {
691+
// Unexpected set
692+
nackReason = E120_NR_UNSUPPORTED_COMMAND_CLASS;
680693
}
681694

682-
} else if ((CmdClass == E120_GET_COMMAND) && (Parameter == SWAPINT(E120_DEVICE_MODEL_DESCRIPTION))) { // 0x0080
683-
if (_rdm.packet.DataLength > 0) {
684-
// Unexpected data
685-
nackReason = E120_NR_FORMAT_ERROR;
686-
} else if (_rdm.packet.SubDev != 0) {
687-
// No sub-devices supported
688-
nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
689-
} else {
690-
// return the DEVICE MODEL DESCRIPTION
691-
_rdm.packet.DataLength = strlen(_initData->deviceModel);
692-
memcpy(_rdm.packet.Data, _initData->deviceModel, _rdm.packet.DataLength);
693-
handled = true;
695+
} else if (Parameter == SWAPINT(E120_DEVICE_MODEL_DESCRIPTION)) { // 0x0080
696+
if (CmdClass == E120_GET_COMMAND) {
697+
if (_rdm.packet.DataLength > 0) {
698+
// Unexpected data
699+
nackReason = E120_NR_FORMAT_ERROR;
700+
} else if (_rdm.packet.SubDev != 0) {
701+
// No sub-devices supported
702+
nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
703+
} else {
704+
// return the DEVICE MODEL DESCRIPTION
705+
_rdm.packet.DataLength = strnlen(_initData->deviceModel, DMXSERIAL_MAX_RDM_STRING_LENGTH);
706+
memcpy(_rdm.packet.Data, _initData->deviceModel, _rdm.packet.DataLength);
707+
handled = true;
708+
}
709+
} else if (CmdClass == E120_SET_COMMAND) {
710+
// Unexpected set
711+
nackReason = E120_NR_UNSUPPORTED_COMMAND_CLASS;
694712
}
695713

696714
} else if (Parameter == SWAPINT(E120_DEVICE_LABEL)) { // 0x0082
@@ -700,7 +718,7 @@ void DMXSerialClass2::_processRDMMessage(byte CmdClass, uint16_t Parameter, bool
700718
nackReason = E120_NR_FORMAT_ERROR;
701719
} else {
702720
memcpy(deviceLabel, _rdm.packet.Data, _rdm.packet.DataLength);
703-
deviceLabel[_rdm.packet.DataLength] = '\0';
721+
deviceLabel[min(_rdm.packet.DataLength, DMXSERIAL_MAX_RDM_STRING_LENGTH)] = '\0';
704722
_rdm.packet.DataLength = 0;
705723
// persist in EEPROM
706724
_saveEEPRom();
@@ -714,24 +732,29 @@ void DMXSerialClass2::_processRDMMessage(byte CmdClass, uint16_t Parameter, bool
714732
// No sub-devices supported
715733
nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
716734
} else {
717-
_rdm.packet.DataLength = strlen(deviceLabel);
735+
_rdm.packet.DataLength = strnlen(deviceLabel, DMXSERIAL_MAX_RDM_STRING_LENGTH);
718736
memcpy(_rdm.packet.Data, deviceLabel, _rdm.packet.DataLength);
719737
handled = true;
720738
}
721739
} // if
722740

723-
} else if ((CmdClass == E120_GET_COMMAND) && (Parameter == SWAPINT(E120_SOFTWARE_VERSION_LABEL))) { // 0x00C0
724-
if (_rdm.packet.DataLength > 0) {
725-
// Unexpected data
726-
nackReason = E120_NR_FORMAT_ERROR;
727-
} else if (_rdm.packet.SubDev != 0) {
728-
// No sub-devices supported
729-
nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
730-
} else {
731-
// return the SOFTWARE_VERSION_LABEL
732-
_rdm.packet.DataLength = strlen(_softwareLabel);
733-
memcpy(_rdm.packet.Data, _softwareLabel, _rdm.packet.DataLength);
734-
handled = true;
741+
} else if (Parameter == SWAPINT(E120_SOFTWARE_VERSION_LABEL)) { // 0x00C0
742+
if (CmdClass == E120_GET_COMMAND) {
743+
if (_rdm.packet.DataLength > 0) {
744+
// Unexpected data
745+
nackReason = E120_NR_FORMAT_ERROR;
746+
} else if (_rdm.packet.SubDev != 0) {
747+
// No sub-devices supported
748+
nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
749+
} else {
750+
// return the SOFTWARE_VERSION_LABEL
751+
_rdm.packet.DataLength = strnlen(_softwareLabel, DMXSERIAL_MAX_RDM_STRING_LENGTH);
752+
memcpy(_rdm.packet.Data, _softwareLabel, _rdm.packet.DataLength);
753+
handled = true;
754+
}
755+
} else if (CmdClass == E120_SET_COMMAND) {
756+
// Unexpected set
757+
nackReason = E120_NR_UNSUPPORTED_COMMAND_CLASS;
735758
}
736759

737760
} else if (Parameter == SWAPINT(E120_DMX_START_ADDRESS)) { // 0x00F0
@@ -822,7 +845,7 @@ void DMXSerialClass2::_processRDMMessage(byte CmdClass, uint16_t Parameter, bool
822845
// Out of range sensor
823846
nackReason = E120_NR_DATA_OUT_OF_RANGE;
824847
} else {
825-
_rdm.packet.DataLength = 13 + strlen(_initData->sensors[sensorNr].description);
848+
_rdm.packet.DataLength = 13 + strnlen(_initData->sensors[sensorNr].description, DMXSERIAL_MAX_RDM_STRING_LENGTH);
826849
_rdm.packet.Data[0] = sensorNr;
827850
_rdm.packet.Data[1] = _initData->sensors[sensorNr].type;
828851
_rdm.packet.Data[2] = _initData->sensors[sensorNr].unit;
@@ -922,12 +945,13 @@ void DMXSerialClass2::_saveEEPRom()
922945
eeprom.sig1 = 0x6D;
923946
eeprom.sig2 = 0x68;
924947
eeprom.startAddress = _startAddress;
925-
strcpy (eeprom.deviceLabel, deviceLabel);
948+
// This needs restricting to 32 chars (potentially no null), for backwards compatibility
949+
strncpy(eeprom.deviceLabel, deviceLabel, DMXSERIAL_MAX_RDM_STRING_LENGTH);
926950
DeviceIDCpy(eeprom.deviceID, _devID);
927951

928952
for (unsigned int i = 0; i < sizeof(eeprom); i++) {
929-
if (((byte *)(&eeprom))[i] != EEPROM.read(i))
930-
EEPROM.write(i, ((byte *)(&eeprom))[i]);
953+
// EEPROM.update does a read/write check and only writes on a change
954+
EEPROM.update(i, ((byte *)(&eeprom))[i]);
931955
} // for
932956
} // _saveEEPRom
933957

‎src/DMXSerial2.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ struct RDMDATA {
9090

9191
byte _TransNo; // transaction number, not checked
9292
byte ResponseType; // ResponseType
93-
byte _unknown; // I don't know, ignore this
93+
byte _MessageCount; // not used unless we support queued messages
9494
uint16_t SubDev; // sub device number (root = 0)
9595
byte CmdClass; // command class
9696
uint16_t Parameter; // parameter ID
@@ -234,10 +234,10 @@ class DMXSerialClass2
234234
/// Returns the Device ID. Copies the UID to the buffer passed through the uid argument.
235235
void getDeviceID(DEVICEID id);
236236

237-
/// Return the current DMX start address that is the first dmx address used by the device.
237+
/// Return the current DMX start address that is the first DMX address used by the device.
238238
uint16_t getStartAddress();
239239

240-
/// Return the current DMX footprint, that is the number of dmx addresses used by the device.
240+
/// Return the current DMX footprint, that is the number of DMX addresses used by the device.
241241
uint16_t getFootprint();
242242

243243
/// Register a device-specific implemented function for RDM callbacks
@@ -252,8 +252,8 @@ class DMXSerialClass2
252252
/// Terminate operation.
253253
void term(void);
254254

255-
/// A short custom label given to the device.
256-
char deviceLabel[DMXSERIAL_MAX_RDM_STRING_LENGTH];
255+
/// A short custom label given to the device. Add an extra char for a null.
256+
char deviceLabel[DMXSERIAL_MAX_RDM_STRING_LENGTH + 1];
257257

258258
/// don't use that method from extern.
259259
void _processRDMMessage(byte CmdClass, uint16_t Parameter, bool8 isHandled);

0 commit comments

Comments
 (0)