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

add tm1829 module to dev branch #984

Closed
wants to merge 3 commits into from
Closed

add tm1829 module to dev branch #984

wants to merge 3 commits into from

Conversation

h3ndrik
Copy link

@h3ndrik h3ndrik commented Jan 28, 2016

This code is taken from the WS2812 module with modified timings and inverted logic levels

see #920: this is a resubmission after the module cleanup
also add some API description (#774)

This code is taken from the WS2812 module with modified timings and inverted logic levels
usage like ws2812 module
@marcelstoer
Copy link
Member

Thanks a lot! I'd like to see two additional things:

@devsaurus in #920 (comment) you mentioned some verification you wanted to perform?

@TerryE
Copy link
Collaborator

TerryE commented Jan 28, 2016

Can I give this a 👎 or at least a hold. We shouldn't be adding more modules which break the Espressif timing guidelines and cause the CPU to crater, until we agree a policy for addressing these issues. In this case interrupts are disabled for ~40N µSec where N is the number of LEDs.

@devsaurus
Copy link
Member

@devsaurus in #920 (comment) you mentioned some verification you wanted to perform?

Not me. In general verification that the code in old-master SDK 0.9.6 is functional with a recent SDK. That's been the reason why the code wasn't pulled into dev as is.
Can be considered as done, now that @h3ndrik rebased the code onto dev.

@Alkorin
Copy link
Contributor

Alkorin commented Jan 29, 2016

Copy-and-paste programming, fonctions comments were wrong in ws2812 module, and ... same here :/

It looks like a similar protocol than ws2812, we could try to share code / use similar implementation.
I don't have any TM1829 here to test :/

@TerryE
Copy link
Collaborator

TerryE commented Jan 30, 2016

The main difference is that the logic is inverted that is each data bit is a low-high rather than high-low, so whilst your encoding scheme can easily be mapped onto the 1xxxxxxxx0 bit sequences output by the uart, this will need to be shifted one sub-bit for the TM1289. We could get this to work, I think, but test HW and an oscilloscope will be needed. Or add a logic inverter in the path.

@marcelstoer
Copy link
Member

@h3ndrik do you intend to fix the code based on the the comments here? If not then please close he PR.

@marcelstoer
Copy link
Member

No feedback in more than two weeks -> closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants