search

hyperion-project / hyperion.ng

The successor to Hyperion aka Hyperion Next Generation
https://hyperion-project.org/
MIT License
3.04k stars 377 forks source link

LedDeviceAdalightApa102.cpp oddness #233

Closed penfold42 closed 7 years ago

penfold42 commented 8 years ago

Are startframesize and endframesize relevant to this driver ? They look like they're lifted from the SPI apa102 driver and I don't think try make sense here. We actually write 6 header bytes (not 4) and we'll have fewer endframesize bytes in the packet that gets sent of rs232.

Also, why are we using rs232 via the adalight driver ? Is it to reuse the timer init code ?

Can we ditch the rewrite code and rely on the smoothing framework to do the continuous send ?

tociek commented 8 years ago

Yes, start frame and end frame are relevant as well as bytes with led count. Please leave them as they are.

WHen I was writing the driver for APA102 via adalight, I had no idea how all of this works, so I wanted to avoid any unnecessary changes comparing to adalight (arduino code is very similar to adalight).

I don't understand your last question so I can't answer it :)

redPanther commented 8 years ago

Can we ditch the rewrite code and rely on the smoothing framework to do the continuous send ?

in theory yes. But currently when smoothing disabled, then we don't have continuous sending. We get to much reports over some strange behaviour regarding smoothing (+color transform). Before we didn't solve the problems, I wouldn't rely on smoothing framework.

penfold42 commented 8 years ago

I get the Ada hi lo checksum bit, but what would be the header and trailing bytes that an apa102 led sees are never sent. Do you have a link to the arduino sketch that receives the adalight apa102 data ?

redPanther commented 8 years ago

https://github.com/hyperion-project/hyperion.ng/blob/master/assets/firmware/arduino/adalightapa102/LightberryHDUSBAPA1021.1.ino it's in the firmware dir. BTW if you have firmware for esp ()udp) than you can add it there

penfold42 commented 8 years ago

Got it - it's clear now. The sketch is basically a dumb rs232 to SPI bridge that waits for the magic Apa header.

We do have a subtle bug in that we're never explicitly sending the leading 4 0x00 bytes nor the trailing 0xff bytes. The apa102 doesn't seem to care, but the sk9822 does seem to need the 0xff series at the end.

I'll submit a PR for testing

redPanther commented 7 years ago

meanwhile adalightapa does directly derived from providerRs232, so this point is cleared, we also added the sketch for reference to our firmware directory.

Whats the open points in this issue? or can we close?

redPanther commented 7 years ago

@penfold42 can we close? especially with my latest pr #305 we have a general led refresh mechanism.

penfold42 commented 7 years ago

i think so - we can open another one if required. I still think we need the trailing 0xffs for the end of frame for correctness