Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switched DMX-library #817

Closed
wants to merge 4 commits into from
Closed

Switched DMX-library #817

wants to merge 4 commits into from

Conversation

EckPhi
Copy link

@EckPhi EckPhi commented Mar 31, 2020

For whatever reason ESP-DMX did not work for me with an ESP32, after some research I discovered some other libraries that had an implementation for ESP8266 and ESP32. I used LXESP32DMX for ESP32 and LXESP8266DMX for ESP8266. Since those are better supported I thought this might be of interest for the whole project as these libraries also support DMX-RDM (not implemented), which might be useful for someone.

According to https://github.com/cansik/esp-dmx-max485 the libraries I use should also be faster.

I did this mainly for myself, so no big deal if the memory usage gain is a deal breaker.

Used LXESP32DMX for ESP32 and LXESP8266DMX for ESP8266
@Aircoookie
Copy link
Owner

@jwingefeld sorry if I overread it somewhere, but what do you think about this library switch? I'd say ESP32 compatiblility and being able to change the pin would be quite nice, especially given that the espdmx pin is the current default LED pin 2.

@EckPhi do you know what the extra direction pin is about?

Happy easter holidays :)

@jwingefeld
Copy link
Contributor

jwingefeld commented Apr 11, 2020

Ohai,

i've found a few issues with that PR.

  1. doesn't compile with the arduino ide. The library is named LXESP8266UARTDMX, the include looks for LXESP8266DMX. Minor problem.
  2. You are calculating the brightness for each individual channel. Every lamp i've seen so far has a dedicated brightness channel. Even the cheap ones from AliExpress. They are often called shutter, brightness, dimmer etc. This would be a double inclusion of the brightness value resulting in a logarithmic scaling. Ideal would be a check whether there's a brightness channel set or not. And then calculate brightness in the color channels, if there's none for that specific fixture. (TL;DR: If !brightnesschannel, then calculateBrightness)
  3. Like you mentioned, the library is three times the size. But then again this is only ~6kb more. Not sure if this is a real problem.
  4. On ESP8266 this does not work at all since you rely on several functions of the ESP32 library, which doesn't get included in the first place.

So in this form, this would break a lot. I would love to see this implemented, just for the performance gain. But on the other hand, the ESPDMX library is simple, straight forward and works great. Even on ESP32 from what i've seen. Maybe we can just improve this one. For example setting the pin without modifying the library itself.

Regarding DMX-RDM: can you give an example what this would be useful for? Lamps usually don't report that much back that could be useful for use inside WLED. I've seen overheat protection or color calibration issues. But you won't see this feature under $1000 USD for a single lamp.

What do you think? You can also contact me on discord. I'm JvPeek#8906 there.

@EckPhi
Copy link
Author

EckPhi commented Apr 11, 2020

@EckPhi do you know what the extra direction pin is about?

@Aircoookie I think this pin controls RE/WE of the RS485 transceiver. However I can't test this, since I'm using one without a RE/WE pin.

doesn't compile with the arduino ide. The library is named LXESP8266UARTDMX, the include looks for LXESP8266DMX. Minor problem.

@jwingefeld To be honest, I didn't have ESP8266 available when I implemented this (I think I tried to compile it, and for whatever reason it succeeded). But I've now fixed it and tested it on an ESP8266.

You are calculating the brightness for each individual channel. Every lamp i've seen so far has a dedicated brightness channel. Even the cheap ones from AliExpress. They are often called shutter, brightness, dimmer etc. This would be a double inclusion of the brightness value resulting in a logarithmic scaling. Ideal would be a check whether there's a brightness channel set or not. And then calculate brightness in the color channels, if there's none for that specific fixture. (TL;DR: If !brightnesschannel, then calculateBrightness)

Actually I implemented a search in the "DMXFixtureMap" for a shutter (5) and I only adjust the color channels if none was found, do I have to search for something else?

On ESP8266 this does not work at all since you rely on several functions of the ESP32 library, which doesn't get included in the first place.

You are right, I have not excluded the semaphore part for ESP8266. (Should be fixed now).

Regarding DMX-RDM: can you give an example what this would be useful for? Lamps usually don't report that much back that could be useful for use inside WLED. I've seen overheat protection or color calibration issues. But you won't see this feature under $1000 USD for a single lamp.

Right, it's pretty useless for lamps... Actually, my intention was that if it is needed in the future (because such lamps become cheaper or something like that), no additional library is needed and that simply the existing library can be used.

Happy Easter :)

@pille
Copy link
Contributor

pille commented Apr 16, 2020

@Aircoookie: did you already made your mind up about merging this? i want to implement a DMX proxy and the new lib provides more flexibility regarding pin configuration.

@Aircoookie
Copy link
Owner

@pille in my opinion this can be merged. But I would still like a go-ahead from @jwingefeld for the improved version as I don't have the hardware and expertise to actually test the DMX output :)

@pille
Copy link
Contributor

pille commented Apr 16, 2020

i can do the testing.

@pille
Copy link
Contributor

pille commented Apr 17, 2020

6410160 still fails to compile because the DMX lib is not found (ESP8266/ESP32)

**************************************************************************
* Looking for LXESP8266UARTDMX.h dependency? Check our library registry!
*
* CLI  > platformio lib search "header:LXESP8266UARTDMX.h"
* Web  > https://platformio.org/lib/search?query=header:LXESP8266UARTDMX.h
*
**************************************************************************

     #include <LXESP8266UARTDMX.h>
                                  ^
compilation terminated.
*** [.pio/build/d1_mini_dmx/src/wled00.ino.cpp.o] Error 1
********************************************************************
* Looking for LXESP32DMX.h dependency? Check our library registry!
*
* CLI  > platformio lib search "header:LXESP32DMX.h"
* Web  > https://platformio.org/lib/search?query=header:LXESP32DMX.h
*
********************************************************************

compilation terminated.
*** [.pio/build/esp32dev/src/wled00.ino.cpp.o] Error 1

@EckPhi
Copy link
Author

EckPhi commented Apr 17, 2020

6410160 still fails to compile because the DMX lib is not found (ESP8266/ESP32)

I did include the libraries manually (for ArduinoIDE https://www.arduino.cc/en/Guide/Libraries#toc5 and for platformio https://docs.platformio.org/en/latest/librarymanager/ldf.html#storage).
However I added the libs as submodule, have a look if it works for you (I'm not using platformio).

@pille
Copy link
Contributor

pille commented Apr 17, 2020

i pulled them in via

git submodule init
git submodule update --recursive

but they don't seem to be picked up automatically in platformio.

when i put the files inside the libs src directory into wled00/src/dependencies/ it compiles.

@Aircoookie
Copy link
Owner

@pille th LXESPDMX libraries are bsd 3 clause licensed. This allows us to include it in the src/dependencies, so let's go that route :) Also makes compilation easier for ArduinoIDE users as they don't have to install the library!

@pille
Copy link
Contributor

pille commented Apr 17, 2020

i did it for ESP8266 in my branch https://github.com/pille/WLED/tree/dmx-lib-switch-fix-esp8622 (based upon 40ec180) so that it compiles.

when i wanted to do the same for ESP32, it got a lot messier, as they bundle rdm lib in different versions and rely on their own libs without bundling them.

note: this PR is still on a branch, before ino-to-cpp-refactoring.

@pille
Copy link
Contributor

pille commented Apr 19, 2020

tested 2294e7e on ESP8266:

  • basically works
  • can't change PIN (setting reset on reboot)
  • occasional flashes (on changing colors)

@pille
Copy link
Contributor

pille commented Apr 22, 2020

decided to base my work for #862 on the proven implementation.

@EckPhi
Copy link
Author

EckPhi commented May 10, 2020

Hi, sorry for the late response, but I was pretty busy in real life. Is there any need to use the LXESP libraries? If so, reimplementing the code should not be a big problem (including the fixes). However, if it is not needed and the ESPDMX-lib works fine with esp8266 and esp32, I would discard this pr.

@Aircoookie Aircoookie added the keep This issue will never become stale/closed automatically label Aug 3, 2020
@Aircoookie
Copy link
Owner

Closing in favor of #2652

@Aircoookie Aircoookie closed this May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep This issue will never become stale/closed automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants