someweisguy
commented
1 year ago Thanks for submitting this. Just so we are on the same page, a pull request is for when you have code that is complete and ready to be merged into the main codebase. That being said, I do have a few comments.
Commit 0f2618d needs to be deleted. It is adding some of your vscode configuration files that are not needed for end users. It is also adding a .gitignore file. I don't necessarily have an issue with that, but the gitignore has src\private\rdm_encode\types.h as an ignored file. I am not sure if this is desired behavior.
I see that you've added a function rdm_encode_response(). It appears to encode just the RDM checksum. I've provided the function rdm_encode_header() which will encode all the header information as well as the checksum, so this function may be a bit redundant. I apologize for not providing very good documentation on the RDM encode/decode functions!
I like your function for rdm_uid_array_is_broadcast()! I think that could be a better way at parsing UIDs when they come in. One of the big headaches is how RDM is transmitted MSB-first and ESP32 stores data LSB-first. I will take a look to see if processing data as an array is better than converting into LSB-first and then processing data.
One of your functions is called set_rdm_muted(). All public functions, types, enums, etc. should begin with either rdm_ or dmx_ so that there aren't any name conflicts; C does not do name mangling like C++ does.
I notice a few of your functions in esp_rdm.c have a few commented out lines of code or are otherwise labelled that they aren't production ready.
Before I can merge this code into the upstream branch, those issues will need to be fixed. If you didn't intend to submit this PR, let me know and I will close this request. All this said, this is shaping up and it's awesome to hear that you got RDM working already!
ebeam-bob
These changes let you implement an RDM Responder, starting with the Discovery process (most of that code was already there) and moving on to support the required Get and Set parameter ID's. However, when tested with a Swisson XMT-350, it can identify the ESP32-based RDM device and get all its Device Info just once per power/reset cycle. After that, the RDM device does not respond to discovery. This code was based on the version of esp_dmx available in the Arduino Library Manager on 12/22/2022, and does not have changes from the base repo after that.