Implement RDM_PID_SUPPORTED_PARAMETERS

[arneboe]

I want to support more parameters, to be able to do that the PID_SUPPORTED_PARAMETERS must be supported.
This is implemented by this PR.
I am not sure that this is the best way to do it. Please feel free to point out any mistakes or strange things. I am not very familiar with your code base and might have overlooked a more obvious solution.

This requires #89 to be merged first

On top of this, I am currently implementing support for PID_DMX_PERSONALITY and will provide a PR within the next 1-2 days :)

[someweisguy]

Thanks for submitting this! Adding support for RDM_PID_SUPPORTED_PARAMETERS is on my to-do list. It is a feature that, in my estimation, is a bit of an edge case so I am thrilled to get some more input from others on how it should be implemented.

As I'm sure you're aware, RDM_PID_SUPPORTED_PARAMETERS interacts with the RDM sub-devices of an RDM responder. RDM responders should send a different response to an RDM_PID_SUPPORTED_PARAMETERS request depending on which sub-device is queried. After considerable thought about the design of this library, I decided that the functions in rdm/responder.h such as rdm_register_device_info() should only support registering the RDM_PID_DEVICE_INFO parameter for the RDM root device. In other words, it is not possible to create an RDM responder which supports the use of sub-devices using functions in rdm/responder.h. The reason for this choice was to make it easier for users to create their own RDM responder; sub-devices would be completely abstracted away from the user. The function rdm_register_parameter() in rdm_utils.h does take a sub_device argument. If an advanced user (such as yourself 😄) wanted to create an RDM responder which supported multiple sub-devices, they would be able to do so by calling this function directly.

The design I had in mind for RDM_PID_SUPPORTED_PARAMETERS was to add two arrays to the dmx_driver_t struct found in dmx/struct.h. One array would contain all the PIDs supported by the root device, and the other array would contain all the PIDs supported by sub-devices. The callback function for RDM_PID_SUPPORTED_PARAMETERS would iterate one of these arrays and emplace the PIDs into the RDM response. PIDs would be added to the appropriate array when rdm_register_parameter() was called.

I haven't looked closely at this PR yet, so if any of this is repetitive, I apologize! I'll review your code and mark it up with notes which hopefully explain the above a bit more clearly.

[someweisguy]

Excellent work! Please let me know your thoughts! :)

[someweisguy]

The way I had designed this library, I had intended RDM_RESPONDER_PIDS_MAX to be the only macro needed to describe the number of supported RDM parameters. Upon reviewing this PR, it now seems like it makes more sense to do it the way you've written it and have two separate macros: RDM_RESPONDER_NUM_PIDS_REQUIRED (which would evaluate to 9 instead of 8) and RDM_RESPONDER_NUM_PIDS_OPTIONAL. The maximum supported number of PIDs would then be RDM_RESPONDER_NUM_PIDS_REQUIRED + RDM_RESPONDER_NUM_PIDS_OPTIONAL. Thoughts?

[arneboe]

Sounds fine.

[arneboe]

As I'm sure you're aware, RDM_PID_SUPPORTED_PARAMETERS interacts with the RDM sub-devices of an RDM responder. RDM responders should send a different response to an RDM_PID_SUPPORTED_PARAMETERS request depending on which sub-device is queried. After considerable thought about the design of this library, I decided that the functions in rdm/responder.h such as rdm_register_device_info() should only support registering the RDM_PID_DEVICE_INFO parameter for the RDM root device. In other words, it is not possible to create an RDM responder which supports the use of sub-devices using functions in rdm/responder.h. The reason for this choice was to make it easier for users to create their own RDM responder; sub-devices would be completely abstracted away from the user. The function rdm_register_parameter() in rdm_utils.h does take a sub_device argument. If an advanced user (such as yourself 😄) wanted to create an RDM responder which supported multiple sub-devices, they would be able to do so by calling this function directly.

I have not put much thought into sub-devices. I just skimmed the part of the specification that deals with sub-devices and am not really sure what to make of it. I have never seen a dmx fixture in the real world that supports this (to be fair I have seen very little fixtures that do support rdm at all).

For my use case the root device hast to support RDM_PID_SUPPORTED_PARAMETERS because of the personalities.
Personalities are a property of the root device. But the getters/setters for personality and personality_description are optional. For optional parameters the rdm master queries RDM_PID_SUPPORTED_PARAMETERS to figure out which parameters are supported. Thus without RDM_PID_SUPPORTED_PARAMETERS on the root level the rdm master will not offer the ability to change the personality to the user (at least my dmx tester behaves that way).

The design I had in mind for RDM_PID_SUPPORTED_PARAMETERS was to add two arrays to the dmx_driver_t struct found in dmx/struct.h. One array would contain all the PIDs supported by the root device, and the other array would contain all the PIDs supported by sub-devices. The callback function for RDM_PID_SUPPORTED_PARAMETERS would iterate one of these arrays and emplace the PIDs into the RDM response. PIDs would be added to the appropriate array when rdm_register_parameter() was called.

That makes perfect sense if rdm_register_parameter() is exposed to the user.

[arneboe]

Thanks for the very detailed answers :) I really appreciate it :)
I will update the PR tomorrow. Too late now :D

[arneboe]

The design I had in mind for RDM_PID_SUPPORTED_PARAMETERS was to add two arrays to the dmx_driver_t struct found in dmx/struct.h. One array would contain all the PIDs supported by the root device, and the other array would contain all the PIDs supported by sub-devices. The callback function for RDM_PID_SUPPORTED_PARAMETERS would iterate one of these arrays and emplace the PIDs into the RDM response. PIDs would be added to the appropriate array when rdm_register_parameter() was called.

In that case we need to store the sub-device id in rdm_cb_table_t as well. Otherwise sub-devices can never register a parameter that is already registered by the root device (e.g. dmx addr).

Edit: This will also clash with how the nvs storage is implemented at the moment.
If a sub-device registeres the same parameter as the root device, the pid will no longer be unique and thus cannot be used as key in dmx_nvs_set

[someweisguy]

Again, great work! I'll go ahead and merge.