Support manufacturer-specific parameters #47

TimMJN · 2023-06-05T19:57:55Z

Hi,

I've been working on implementing manufacturer-specific parameters (PID 0x8000- 0xFFDF), including E120_GET_COMMAND, E120_SET_COMMAND and E120_PARAMETER_DESCRIPTION.

Right now, the parameter values are simply taken from / dumped in a array of bytes and processed in a user-callback function. It's up to the user to store/process/whatever the values.

type fix

header

add example for parameters

mathertel · 2023-06-06T06:12:51Z

That's a great improvement. Thanks a lot.
I will test...

peternewman

Looks good to me, some mostly trivial comments.

If you've implemented manufacturer specific PIDs, it would be good to get them added to the OLA PID store here:
http://rdm.openlighting.org/pid/manufacturer
Via:
https://github.com/OpenLightingProject/rdm-app/blob/master/data/pid_data.py#L1

Also N.B. that manufacturer specific PIDs are supposed to be unique per manufacturer, there should probably be a further warning so people don't reuse those themself!

Ideally somewhere you should check the manufacturer specific PIDs are in the correct range (0x8000-?).

Likewise if neither get nor set are supported.

src/DMXSerial2.h

examples/RDMSerialRecvParameter/RDMSerialRecvParameter.ino

src/DMXSerial2.h

peternewman · 2023-06-06T14:15:42Z

src/DMXSerial2.cpp

+          uint8_t sensorNr = _rdm.packet.Data[0];
+          if (sensorNr >= _initData->sensorsLength) {


Sensor numbers don't (currently) have to be contiguous IIRC.

currently the sensornumber is tied to the index in the sensors struct, so they will always be contiguous. Is there a reason to change this? E.g. are specific sensor numbers tied to specific sensor types?

currently the sensornumber is tied to the index in the sensors struct, so they will always be contiguous. Is there a reason to change this?

Fair point, no, no specific reason.

E.g. are specific sensor numbers tied to specific sensor types?

No there's no technical reason for it.

src/DMXSerial2.cpp

src/DMXSerial2.h

peternewman · 2023-06-06T14:26:27Z

examples/RDMSerialRecv/RDMSerialRecv.ino

+// 04.06.2023 Tim Nijssen: Add support for device-specific parameters
+// 05.06.2023 Tim Nijssen: integrate sensors example
+// 05.06.2023 Tim Nijssen: Add example for device-specific parameters


I genuinely don't know, and a lot of other stuff has been duplicated, but should these comments be in this ino file given they aren't relevant to it?

I'll leave this one for @mathertel to decide on

peternewman · 2023-06-06T14:33:42Z

Have you run the OLA RDM Tests against this too?
https://www.openlighting.org/rdm-tools/rdm-responder-tests/
https://www.openlighting.org/rdm-tools/rdm-responder-tests/running-the-tests/

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

TimMJN · 2023-06-06T22:00:37Z

@peternewman

Thanks for the review! I've processed most of your comments.

I don't think the library should enforce using unique manufacturer-specific PIDs, but perhaps @mathertel could add a remark on this on his website, similarly to how he mentions unique manufacturer codes.

Regarding the RDM test, working on it.. I'm having some python3 compatibility issues with running the tests.

TimMJN · 2023-06-06T22:06:51Z

To do: add support for cold reset using RESET_DEVICE 0x01

void(* resetFunc) (void) = 0;
resetFunc()

(leave the warm reset RESET_DEVICE 0xFF to be user-defined)

peternewman · 2023-06-06T22:14:49Z

@peternewman

Thanks for the review! I've processed most of your comments.

Great thanks.

I don't think the library should enforce using unique manufacturer-specific PIDs, but perhaps @mathertel could add a remark on this on his website, similarly to how he mentions unique manufacturer codes.

I don't think the library can technically enforce it (unless PIDs had to be centrally registered in a list of defines or something), so a note/remark is all we can manage. It shouldn't actually be an issue until two individual devices meet. But I guess it will stop someone tripping themselves up by not appreciating the subtlety of how the system should work.

Regarding the RDM test, working on it.. I'm having some python3 compatibility issues with running the tests.

Our 0.10.9 release or the 0.10 or master branches should all solve the Python 3 issues.

TimMJN · 2023-06-07T20:36:27Z

RDM test results:

OLA RDM Responder Tests

GetMaxPacketSize: FAILED
Check if the responder can handle a packet of the maximum size.
 GET: uid: 0987:2012fb3b, pid: DEVICE_INFO (0x0060), sub device: 0, data: b'\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f !"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~\x7f\xc2\x80\xc2\x81\xc2\x82\xc2\x83\xc2\x84\xc2\x85\xc2\x86\xc2\x87\xc2\x88\xc2\x89\xc2\x8a\xc2\x8b\xc2\x8c\xc2\x8d\xc2\x8e\xc2\x8f\xc2\x90\xc2\x91\xc2\x92\xc2\x93\xc2\x94\xc2\x95\xc2\x96\xc2\x97\xc2\x98\xc2\x99\xc2\x9a\xc2\x9b\xc2\x9c\xc2\x9d\xc2\x9e\xc2\x9f\xc2\xa0\xc2\xa1\xc2\xa2\xc2\xa3\xc2\xa4\xc2\xa5\xc2\xa6\xc2\xa7\xc2\xa8\xc2\xa9\xc2\xaa\xc2\xab\xc2\xac\xc2\xad\xc2\xae\xc2\xaf\xc2\xb0\xc2\xb1\xc2\xb2\xc2\xb3\xc2\xb4\xc2\xb5\xc2\xb6\xc2\xb7\xc2\xb8\xc2\xb9\xc2\xba\xc2\xbb\xc2\xbc\xc2\xbd\xc2\xbe\xc2\xbf\xc3\x80\xc3\x81\xc3\x82\xc3\x83\xc3\x84\xc3\x85\xc3\x86\xc3\x87\xc3\x88\xc3\x89\xc3\x8a\xc3\x8b\xc3\x8c\xc3\x8d\xc3\x8e\xc3\x8f\xc3\x90\xc3\x91\xc3\x92\xc3\x93\xc3\x94\xc3\x95\xc3\x96\xc3\x97\xc3\x98\xc3\x99\xc3\x9a\xc3\x9b\xc3\x9c\xc3\x9d\xc3\x9e\xc3\x9f\xc3\xa0\xc3\xa1\xc3\xa2\xc3\xa3\xc3\xa4\xc3\xa5\xc3\xa6'
 Response status: Failed to send request
 Failed: expected one of:
  CC: Get, PID 0x0060, NACK RDMNack(value=1, desc="Format Error")
  CC: Get, PID 0x0060, NACK RDMNack(value=8, desc="Packet size unsupported")
  CC: Get, PID 0x0060, ACK, fields [], values {}
  RDM_INVALID_RESPONSE
  RDM_TIMEOUT

SetEmptyDeviceLabel: FAILED
SET the device label with no data.
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['']
 Response status: Response Timeout
 Failed: expected one of:
  CC: Set, PID 0x0082, NACK RDMNack(value=4, desc="Write protect")
  CC: Set, PID 0x0082, NACK RDMNack(value=5, desc="Unsupported command class")
  CC: Set, PID 0x0082, ACK, fields [], values {}
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['this is a string with 32 charact']
 Response status: Response Timeout

SetNonPrintableAsciiDeviceLabel: FAILED
SET the device label to something that contains non-printable ASCII
     characters.
  
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['str w\\r non\\x1bprint ASCII\\x7f']
 Response status: Response Timeout
 Failed: expected one of:
  CC: Set, PID 0x0082, NACK RDMNack(value=4, desc="Write protect")
  CC: Set, PID 0x0082, NACK RDMNack(value=6, desc="Data out of range")
  CC: Set, PID 0x0082, NACK RDMNack(value=1, desc="Format Error")
  CC: Set, PID 0x0082, NACK RDMNack(value=5, desc="Unsupported command class")
  CC: Set, PID 0x0082, ACK, fields [], values {}
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['this is a string with 32 charact']
 Response: RDMResponse(type=ACK, command_class=SET), PID: 0x0082, TN: 53, Error: Expected 0 bytes but got 32
 Failed: Invalid Param data: Expected 0 bytes but got 32

SetFullSizeDeviceLabel: FAILED
SET the device label.
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['this is a string with 32 charact']
 Response status: Response Timeout
 Failed: expected one of:
  CC: Set, PID 0x0082, NACK RDMNack(value=4, desc="Write protect")
  CC: Set, PID 0x0082, NACK RDMNack(value=5, desc="Unsupported command class")
  CC: Set, PID 0x0082, ACK, fields [], values {}
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['this is a string with 32 charact']
 Response status: Response Timeout

SetDeviceLabel: FAILED
SET the device label.
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['test label']
 Response status: Response Timeout
 Failed: expected one of:
  CC: Set, PID 0x0082, NACK RDMNack(value=4, desc="Write protect")
  CC: Set, PID 0x0082, NACK RDMNack(value=5, desc="Unsupported command class")
  CC: Set, PID 0x0082, ACK, fields [], values {}
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['this is a string with 32 charact']
 Response status: Response Timeout

------------------- Summary --------------------
OLA Version: 0.10.9
Test Run: 2023-06-07 10:33:28 PM 
UID: 0987:2012fb3b
Manufacturer: mathertel.de
Model Description: Arduino RDM Device
Software Version: 16777216
------------------- Warnings --------------------
------------------ Advisories -------------------
----------------- By Category -------------------
              Configuration:     7 /   7    100%
                    Control:    19 /  19    100%
         Core Functionality:     4 /   4    100%
               DMX512 Setup:    22 /  22    100%
            Dimmer Settings:    13 /  13    100%
           Display Settings:     2 /   2    100%
           Error Conditions:   312 / 313    100%
   IP and DNS Configuration:     5 /   5    100%
         Network Management:    25 /  25    100%
      Power / Lamp Settings:     9 /   9    100%
        Product Information:    15 /  19    79%
            RDM Information:     1 /   1    100%
                    Sensors:     5 /   5    100%
          Status Collection:     3 /   3    100%
                Sub Devices:    76 /  76    100%
-------------------------------------------------
523 / 583 tests run, 518 passed, 5 failed, 0 broken

TimMJN · 2023-06-07T20:54:47Z

Seems like we should add a catch for Set on the device label, return nack Write protected.

The first fail is related to #46?

TimMJN · 2023-06-10T12:59:20Z

found the problem with device labels. Writing up to 32 characters to EEPROM takes too long to send the response in time. I've moved the _saveEEPRom call after the response has been sent. This still fails some tests, as it now takes too long to read back the results immediately after setting a new label. Unless @mathertel wants to eg rewrite the EEPROM saving to a background process that writes max 1 byte per tick, this is what it is.

TimMJN · 2023-06-10T13:06:38Z

Test output:

OLA RDM Responder Tests

GetMaxPacketSize: FAILED
Check if the responder can handle a packet of the maximum size.
 GET: uid: 0987:2012fb3b, pid: DEVICE_INFO (0x0060), sub device: 0, data: b'\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f !"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~\x7f\xc2\x80\xc2\x81\xc2\x82\xc2\x83\xc2\x84\xc2\x85\xc2\x86\xc2\x87\xc2\x88\xc2\x89\xc2\x8a\xc2\x8b\xc2\x8c\xc2\x8d\xc2\x8e\xc2\x8f\xc2\x90\xc2\x91\xc2\x92\xc2\x93\xc2\x94\xc2\x95\xc2\x96\xc2\x97\xc2\x98\xc2\x99\xc2\x9a\xc2\x9b\xc2\x9c\xc2\x9d\xc2\x9e\xc2\x9f\xc2\xa0\xc2\xa1\xc2\xa2\xc2\xa3\xc2\xa4\xc2\xa5\xc2\xa6\xc2\xa7\xc2\xa8\xc2\xa9\xc2\xaa\xc2\xab\xc2\xac\xc2\xad\xc2\xae\xc2\xaf\xc2\xb0\xc2\xb1\xc2\xb2\xc2\xb3\xc2\xb4\xc2\xb5\xc2\xb6\xc2\xb7\xc2\xb8\xc2\xb9\xc2\xba\xc2\xbb\xc2\xbc\xc2\xbd\xc2\xbe\xc2\xbf\xc3\x80\xc3\x81\xc3\x82\xc3\x83\xc3\x84\xc3\x85\xc3\x86\xc3\x87\xc3\x88\xc3\x89\xc3\x8a\xc3\x8b\xc3\x8c\xc3\x8d\xc3\x8e\xc3\x8f\xc3\x90\xc3\x91\xc3\x92\xc3\x93\xc3\x94\xc3\x95\xc3\x96\xc3\x97\xc3\x98\xc3\x99\xc3\x9a\xc3\x9b\xc3\x9c\xc3\x9d\xc3\x9e\xc3\x9f\xc3\xa0\xc3\xa1\xc3\xa2\xc3\xa3\xc3\xa4\xc3\xa5\xc3\xa6'
 Response status: Failed to send request
 Failed: expected one of:
  CC: Get, PID 0x0060, NACK RDMNack(value=1, desc="Format Error")
  CC: Get, PID 0x0060, NACK RDMNack(value=8, desc="Packet size unsupported")
  CC: Get, PID 0x0060, ACK, fields [], values {}
  RDM_INVALID_RESPONSE
  RDM_TIMEOUT

SetNonPrintableAsciiDeviceLabel: FAILED
SET the device label to something that contains non-printable ASCII
     characters.
  
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['str w\\r non\\x1bprint ASCII\\x7f']
 Response: RDMResponse(type=ACK, command_class=SET), PID: 0x0082, TN: 117, PDL: 0, data: {}
 GET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: []
 Response status: Response Timeout
 Failed: expected one of:
  CC: Get, PID 0x0082, ACK, fields ['label'], values {}
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['somethingelse']
 Response: RDMResponse(type=ACK, command_class=SET), PID: 0x0082, TN: 119, PDL: 0, data: {}

SetFullSizeDeviceLabel: FAILED
SET the device label.
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['this is a string with 32 charact']
 Response: RDMResponse(type=ACK, command_class=SET), PID: 0x0082, TN: 205, PDL: 0, data: {}
 GET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: []
 Response status: Response Timeout
 Failed: expected one of:
  CC: Get, PID 0x0082, ACK, fields ['label'], values {}
 SET: uid: 0987:2012fb3b, pid: DEVICE_LABEL (0x0082), sub device: 0, args: ['somethingelse']
 Response status: Response Timeout

SetZeroDMXStartAddress: FAILED
Check the DMX address can't be set to 0.
 SET: pid: DMX_START_ADDRESS (0x00f0), sub device: 0, data: b'\x00\x00'
 Response status: Response Timeout
 Failed: expected one of:
  CC: Set, PID 0x00f0, NACK RDMNack(value=6, desc="Data out of range")

------------------- Summary --------------------
OLA Version: 0.10.9
Test Run: 2023-06-10 03:03:15 PM 
UID: 0987:2012fb3b
Manufacturer: mathertel.de
Model Description: Arduino RDM Device
Software Version: 16777216
------------------- Warnings --------------------
------------------ Advisories -------------------
----------------- By Category -------------------
              Configuration:     7 /   7    100%
                    Control:    19 /  19    100%
         Core Functionality:     4 /   4    100%
               DMX512 Setup:    22 /  22    100%
            Dimmer Settings:    13 /  13    100%
           Display Settings:     2 /   2    100%
           Error Conditions:   311 / 313    99%
   IP and DNS Configuration:     5 /   5    100%
         Network Management:    25 /  25    100%
      Power / Lamp Settings:     9 /   9    100%
        Product Information:    19 /  21    90%
            RDM Information:     1 /   1    100%
                    Sensors:     5 /   5    100%
          Status Collection:     3 /   3    100%
                Sub Devices:    76 /  76    100%
-------------------------------------------------
525 / 583 tests run, 521 passed, 4 failed, 0 broken

notice that SetZeroDMXStartAddress was ran directly after SetFullSizeDeviceLabel, causing the timeout.

peternewman

Some more improvements and comments

examples/RDMSerialRecvParameter/RDMSerialRecvParameter.ino

src/DMXSerial2.h

peternewman · 2023-06-11T16:46:40Z

src/DMXSerial2.cpp

+  if (updateEEPRom)
+    _saveEEPRom();


See also my comments in the main discussion.

peternewman · 2023-06-11T16:51:33Z

src/DMXSerial2.cpp

+          uint8_t sensorNr = _rdm.packet.Data[0];
+          if (sensorNr >= _initData->sensorsLength) {


currently the sensornumber is tied to the index in the sensors struct, so they will always be contiguous. Is there a reason to change this?

Fair point, no, no specific reason.

E.g. are specific sensor numbers tied to specific sensor types?

No there's no technical reason for it.

src/DMXSerial2.cpp

peternewman · 2023-06-11T17:06:01Z

Seems like we should add a catch for Set on the device label, return nack Write protected.

No, there's little point implementing a read only device label, but I think you've got to the bottom of that.

The first fail is related to #46?

I thought we'd fixed that before a few times.

found the problem with device labels. Writing up to 32 characters to EEPROM takes too long to send the response in time.

Agreed.

I've moved the _saveEEPRom call after the response has been sent. This still fails some tests, as it now takes too long to read back the results immediately after setting a new label. Unless @mathertel wants to eg rewrite the EEPROM saving to a background process that writes max 1 byte per tick, this is what it is.

However I believe the correct fix is to implement an Ack Timer, which tells the controller to try again in a time you specify when the device should be ready. I think technically it shouldn't ack until it's actually completed the write (e.g. consider if there's a power cut before it finishes writing, the controller will think the write succeeded when it didn't. Obviously the balance is implementation complexity, so TBH as long as the DMX addresses ones work it may be easiest to just revert that change and put it in the two hard pile, or at least push it to another PR.

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

TimMJN · 2023-06-11T20:44:19Z

No, there's little point implementing a read only device label, but I think you've got to the bottom of that.

You're right, I was misreading the test output.

However I believe the correct fix is to implement an Ack Timer, which tells the controller to try again in a time you specify when the device should be ready. I think technically it shouldn't ack until it's actually completed the write (e.g. consider if there's a power cut before it finishes writing, the controller will think the write succeeded when it didn't. Obviously the balance is implementation complexity, so TBH as long as the DMX addresses ones work it may be easiest to just revert that change and put it in the two hard pile, or at least push it to another PR.

I have one issue with the current (before my changes) method. Correct me if I'm wrong, but the delayed ACK message would be able to mangle up other communication in the universe happening at that time, right? Queued messages are not implemented right now, but how about this:

When a SET on the device label is received, we compare the new label to the existing one.
If it is identical, we issue an ACK.
If it's not, we issue an ACK_TIMER (300ms should be sufficient), then rewrite the EEPROM.

That way, we keep queued messages on the to-do list, but the controller can re-issue the SET and will receive an ACK.

peternewman

A few more comments/tweaks/improvements

peternewman · 2023-06-27T20:36:37Z

examples/RDMSerialRecvParameter/RDMSerialRecvParameter.ino

+  } else {
+    return false;
+  }
+  return true;


Really, when we've not done anything. Won't that silence unknown PID type stuff?

peternewman · 2023-06-27T20:43:29Z

examples/RDMSerialRecvParameter/RDMSerialRecvParameter.ino

+        *nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
+      } else {
+
+        unsigned long seconds = millis() / 1000;


While it's nice you've added some real data, I'd say the device hours field is where this value should be.
DEVICE_HOURS = "This parameter is used to retrieve or set the number of hours of operation the device has been in
use."
LAMP_HOURS = "This parameter is used to retrieve the number of lamp hours or to set the counter in the device to
a specific starting value."

peternewman · 2023-06-27T20:44:00Z

examples/RDMSerialRecvParameter/RDMSerialRecvParameter.ino

+        rdm->Data[0] = (seconds & 0xff000000) >> 24;
+        rdm->Data[1] = (seconds & 0x00ff0000) >> 16;
+        rdm->Data[2] = (seconds & 0x0000ff00) >> 8;
+        rdm->Data[3] = seconds & 0x000000ff;


Use the existing WRITEINT32 macro, or add one if its missing.

peternewman · 2023-06-27T20:45:45Z

examples/RDMSerialRecvParameter/RDMSerialRecvParameter.ino

+#if defined(SERIAL_DEBUG)
+    Serial.println("no signal since 30 secs.");
+#endif
+    // Show default color, when no data was received since 30 seconds or more.


Improve English (probably not your issue), but would you mind fixing it elsewhere too please?

Suggested change

// Show default color, when no data was received since 30 seconds or more.

// Show default color, when no data was received for 30 seconds or more.

peternewman · 2023-06-27T20:45:56Z

examples/RDMSerialRecvParameter/RDMSerialRecvParameter.ino

+
+  } else {
+#if defined(SERIAL_DEBUG)
+    Serial.println("no signal since 30 secs.");


Suggested change

Serial.println("no signal since 30 secs.");

Serial.println("no signal for 30 secs.");

peternewman · 2023-06-27T20:46:38Z

examples/RDMSerialRecvParameter/RDMSerialRecvParameter.ino

+
+
+void loop() {
+  // Calculate how long no data backet was received


Suggested change

// Calculate how long no data backet was received

// Find how long ago last data packet was received

peternewman · 2023-06-27T20:52:05Z

examples/RDMSerialRecvParameter/RDMSerialRecvParameter.ino

+        rdm->Data[0] = 0;
+        rdm->Data[1] = 0;
+        rdm->Data[2] = 2;
+        rdm->Data[3] = 0;


Use the existing macro or add one if missing.

peternewman · 2023-06-27T20:53:56Z

src/DMXSerial2.cpp

@@ -605,14 +622,15 @@ void DMXSerialClass2::term(void)
 void DMXSerialClass2::_processRDMMessage(byte CmdClass, uint16_t Parameter, bool8 handled, bool8 doRespond)
 {
  uint16_t nackReason = E120_NR_UNKNOWN_PID;
+  bool8 updateEEPRom = false;


Personally I'd probably just camel case as updateEeprom, but if not, EEPROM is an acronym of acronyms, ROM is an acronym so itself should remain capitalised surely?

src/DMXSerial2.cpp

peternewman · 2023-06-27T21:07:59Z

src/DMXSerial2.cpp

+                int16_t recordedValue = 0;
+                bool8 res = false;
+                if (_sensorFunc) {
+                  res = _sensorFunc(i, &sensorValue, &lowestValue, &highestValue, &recordedValue);


We need to do something clever here if we're in 0xff mode. Ideally ANDing all the results, but obviously not the default initialisation...

Tracking failure would mean we could OR it I think.

peternewman · 2023-06-27T21:19:08Z

I have one issue with the current (before my changes) method. Correct me if I'm wrong, but the delayed ACK message would be able to mangle up other communication in the universe happening at that time, right? Queued messages are not implemented right now, but how about this:

You don't send it gratuitously, the controller will wait and then re-query it when the timeout has expired. IIRC queued messages are mostly about things like when I change the DMX address via a local UI, or temperature sensor readings and stuff.

* When a SET on the device label is received, we compare the new label to the existing one.

* If it is identical, we issue an ACK.

* If it's not, we issue an ACK_TIMER (300ms should be sufficient), then rewrite the EEPROM.
That way, we keep queued messages on the to-do list, but the controller can re-issue the SET and will receive an ACK.

Yeah I think that ought to work, but it will want a chunk more testing.

Could I suggest reverting the changes around this from this PR, which is already quite big and complicated, and pushing it into a separate PR. Realistically there will be other equally buggy responders, so controllers probably deal with them already, which means this is mostly just about making us more compliant and allowing us to pass more tests. Which is a great thing to do, but it would be a shame if it delayed the addition of useful functionality to people in the mean time.

TimMJN added 6 commits June 4, 2023 22:49

Update rdm.h

ff45034

type fix

implement parameters

6fce9a3

move into dir

120aa1c

integrate sensors example

779518c

Update RDMSerialRecv.ino

33b380d

header

Create RDMSerialRecvParameter.ino

206e07b

add example for parameters

TimMJN added 4 commits June 6, 2023 13:46

Update RDMSerialRecvParameter.ino

1b629b0

Update RDMSerialRecvSensor.ino

c95a9c2

Update RDMSerialRecv.ino

89330b8

Update DMXSerial2.h

b676b51

peternewman suggested changes Jun 6, 2023

View reviewed changes

TimMJN and others added 13 commits June 6, 2023 19:31

Update src/DMXSerial2.cpp

7a04e00

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

Update src/DMXSerial2.cpp

7f5dc82

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

Update src/DMXSerial2.h

df16351

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

Update src/DMXSerial2.h

d2678fc

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

Update src/DMXSerial2.h

bde3c2a

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

change WRITELONG to WRITEINT32

ee74c82

Update src/DMXSerial2.h

d7e9214

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

Update src/DMXSerial2.h

121e625

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

Update src/DMXSerial2.h

2e6551a

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

Update src/DMXSerial2.cpp

fb0c8e1

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

update nackReason for unknown manufacturer PID

1081219

change parameterNr to parameterIndex, add pid to callback functions

f4ed6d8

support sensor (re)set, return 0x0000 for unsupported sensor functions

ec99b3e

bugfixes

092aac0

TimMJN added 2 commits June 10, 2023 14:33

added a catch for non-ascii characters in the device label

8048876

move _saveEEPRom after response has been sent

e04ebe4

peternewman suggested changes Jun 11, 2023

View reviewed changes

TimMJN and others added 6 commits June 11, 2023 22:00

Update src/DMXSerial2.h

b55fd58

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

Update src/DMXSerial2.h

e05abdc

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

implement resetting all sensors

0c924d5

Merge branch 'master' of https://github.com/TimMJN/DmxSerial2

1b261ac

Update RDMSerialRecvParameter.ino

79f2c8c

correct terminilogy

05f46d9

TimMJN changed the title ~~Support device-specific parameters~~ Support manufacturer-specific parameters Jun 11, 2023

typo

3d5bacd

peternewman suggested changes Jun 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support manufacturer-specific parameters #47

Support manufacturer-specific parameters #47

TimMJN commented Jun 5, 2023 •

edited

Loading

mathertel commented Jun 6, 2023

peternewman left a comment

peternewman Jun 6, 2023

TimMJN Jun 6, 2023

peternewman Jun 11, 2023

peternewman Jun 6, 2023

TimMJN Jun 6, 2023

peternewman commented Jun 6, 2023

TimMJN commented Jun 6, 2023

TimMJN commented Jun 6, 2023

peternewman commented Jun 6, 2023

TimMJN commented Jun 7, 2023 •

edited

Loading

TimMJN commented Jun 7, 2023

TimMJN commented Jun 10, 2023

TimMJN commented Jun 10, 2023

peternewman left a comment

peternewman Jun 11, 2023

peternewman Jun 11, 2023

peternewman commented Jun 11, 2023

TimMJN commented Jun 11, 2023

peternewman left a comment

peternewman Jun 27, 2023

peternewman Jun 27, 2023

peternewman Jun 27, 2023

peternewman Jun 27, 2023

peternewman Jun 27, 2023

peternewman Jun 27, 2023

peternewman Jun 27, 2023

peternewman Jun 27, 2023

peternewman Jun 27, 2023

peternewman commented Jun 27, 2023

		uint8_t sensorNr = _rdm.packet.Data[0];
		if (sensorNr >= _initData->sensorsLength) {

	// Show default color, when no data was received since 30 seconds or more.
	// Show default color, when no data was received for 30 seconds or more.

	Serial.println("no signal since 30 secs.");
	Serial.println("no signal for 30 secs.");



		void loop() {
		// Calculate how long no data backet was received

	// Calculate how long no data backet was received
	// Find how long ago last data packet was received

Support manufacturer-specific parameters #47

Are you sure you want to change the base?

Support manufacturer-specific parameters #47

Conversation

TimMJN commented Jun 5, 2023 • edited Loading

mathertel commented Jun 6, 2023

peternewman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternewman commented Jun 6, 2023

TimMJN commented Jun 6, 2023

TimMJN commented Jun 6, 2023

peternewman commented Jun 6, 2023

TimMJN commented Jun 7, 2023 • edited Loading

TimMJN commented Jun 7, 2023

TimMJN commented Jun 10, 2023

TimMJN commented Jun 10, 2023

peternewman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternewman commented Jun 11, 2023

TimMJN commented Jun 11, 2023

peternewman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternewman commented Jun 27, 2023

TimMJN commented Jun 5, 2023 •

edited

Loading

TimMJN commented Jun 7, 2023 •

edited

Loading