someweisguy / esp_dmx

Espressif ESP32 implementation of ANSI-ESTA E1.11 DMX-512A and E1.20 RDM
MIT License
335 stars 35 forks source link

Commit f148f7b (fix idf v5 timer error) breaks dmx_receive and rdm #86

Closed arneboe closed 1 year ago

arneboe commented 1 year ago

I am tracking release/v3.1 in my current wled development. Thats why I noticed that the commit https://github.com/someweisguy/esp_dmx/commit/f148f7bacedc0d43afb692755913ced460714a1d breaks dmx_receive. I can no longer receive any dmx data with this commit. Without this commit everything works fine.

I compiled the minimal arduino example to verify that the bug is not on my side.

 - framework-arduinoespressif32 @ 3.20005.220925 (2.0.5) 
 - toolchain-xtensa-esp32 @ 8.4.0+2021r2-patch3
someweisguy commented 1 year ago

Thanks for opening this issue. I am aware of the problem and am working to resolve it! I have been slow to do so because I just moved across the country so it has been difficult to find time to do more debugging. :)

This issue appears to be related to PR #80, which was intended to allow for this library to be ported to different microcontrollers. The commit you mention, f148f7b, fixed an issue with dmx_send() as mentioned in issue #83, but is now causing an issue with dmx_receive(). It appears that the issue is caused by this line. Setting the field at that line to true fixes dmx_receive(), but breaks dmx_send() and vice versa.

I'll investigate further!

someweisguy commented 1 year ago

Aha! It looks like it was a simple mistake with how the dmx_receive() function was notifying the DMX driver that a task was awaiting data. It appears that this may have been fixed in 6534cf6. I'll do some more testing before closing this issue.

Give the latest commit a try and let me know if you run into any more issues! :)

arneboe commented 1 year ago

Thanks for the quick response :-) The latest commit fixed it. Thank you!!

arneboe commented 1 year ago

Looks like it also broke rdm. Starting from f148f7b I do no longer get rdm responses on my rdm tester. This is still broken in the last commit.

someweisguy commented 1 year ago

Thanks for the feedback! I am successfully using RDM on Arduino with two ESP32s on the ESP-IDF.

Are you using Arduino? I see that that I am unable to process RDM on Arduino.

arneboe commented 1 year ago

I am using your minimal responder example with platformio and the arduino framework on an ESP32-32D as dmx device and the Botex Dr. RDM I DMX RDM Tester as rdm master.

someweisguy commented 1 year ago

It appears that the issue is caused by this line. Setting the field at that line to true fixes dmx_receive(), but breaks dmx_send() and vice versa.

I'm revisiting this comment. It looks like I was chasing my own tail for a bit here. The issue with dmx_receive() appears to have been an issue with the task notification as I mentioned above. The issue with dmx_send() was created when I set the field in the above line to false. I undid this and it appears RDM is working on Arduino again.

EDIT: But now ESP-IDF sending is broken again! 🙃

someweisguy commented 1 year ago

Okay, I believe I have finally fixed this. It was an issue with the auto_reload flag in the ESP32 hardware timer. In this library, the hardware timer is used for many purposes, including generating the DMX break and MAB, as well as timing the RDM bus to ensure that requests and responses are sent only when they are allowed to be sent. The auto_reload flag tells the hardware timer to automatically reset its counter back to 0 when the timer has finished counting down to its alarm value.

Between ESP-IDF v4 (which is what Arduino still uses, since Arduino has not updated their ESP32 framework yet) and ESP-IDF v5, the driver for the hardware timer changed. In the v4 hardware timer driver, the auto_reload flag is set once when initializing the hardware timer. In the v5 driver, the auto_reload flag is set whenever setting a new alarm for the hardware timer. In v4, the auto_reload flag is set to true. In v5, the auto_reload flag is set to false except when using the timer to send the DMX break.

For some reason setting the auto_reload flag to true at all times in v5 causes the problem we are seeing. This is the behavior I found even though setting auto_reload to true at all times in v4 does not cause a problem. I can only pray to the ESP32 gods that this isn't a bug in the v5 driver that will be fixed in a later ESP-IDF version. Otherwise, the HAL layer will become much more complicated.

This has been fixed and merged from #91. Please reopen this issue if you see any further problems!