Added support for tm1829 based led strips #1297

sebi2k1 · 2016-05-16T08:39:15Z

Squashed into single commit
Added module documentation
Rebased on dev

devsaurus · 2016-05-16T09:45:34Z

Thanks for the contribution @sebi2k1.
I don't have any TM1829 hardware available but since rework of the WS2812 driver finished recently with #1263, I'd like to review this PR along the learnings we gathered there.

Locking interrupts vs stability

The driver locks interrupts - will it lock them for more than 50 µs? Doing so can impact the overall system stability as discussed in #902 (comment). Also see How does the non-OS SDK structure execution for details regarding the timing requirements.

In case it compromises stability then please investigate for other techniques than bit-banging to implement the low-level protocol. Eg WS2812 uses UART1 on GPIO2 to decouple the CPU & system latencies from bit stream generation (introduced with #951). It's now considered compliant with the SDK guidelines.

Lua API

As far as I can see, this driver adopts a similar API than WS2812's previous variant. WS2812 was enhanced by a buffer sub-module (see #902 and #1263) that could also be applicable for TM1829. Do you see a change to adapt this as well (maybe in a second iteration)?

marcelstoer · 2016-05-16T10:20:03Z

app/include/user_modules.h

@@ -58,6 +58,7 @@
 #define LUA_USE_MODULES_WIFI
 //#define LUA_USE_MODULES_WS2801
 //#define LUA_USE_MODULES_WS2812
+#define LUA_USE_MODULES_TM1829


New modules need to be disabled by default and ordered alphabetically.

marcelstoer · 2016-05-16T10:23:13Z

As per https://github.com/nodemcu/nodemcu-firmware/blob/dev/CONTRIBUTING.md#writing-documentation the documentation page needs to be registered in mkdocs.yml.

marcelstoer · 2016-05-16T10:30:41Z

Maybe worth noting that we had a (discarded) TM1829 contribution before: #984.

sebi2k1 · 2016-05-16T12:46:54Z

@marcelstoer My initial approach was similar to #984 (adapt timings, invert levels), but it failed due to some timing issue. I've optimised the algorithm to reduce memory access during timing critical section in favor of increased register use. I could verify the timing for 80/160MHz using an DSO and on my led strip.

Regarding bit bang vs UART. One bitstream takes roughly (2.4us * 24bits + 6us = 63,6us). So depending how much leds are in the daisy chain it might take much longer than 50us (e.g. 3ms on my 50 led controller strip). It might work to disable/enable ISR only when writing a single byte, longer gaps between each byte are fine if < 500us.

@devsaurus I already noticed the buffer implementation and I'm interested in adding support. For my current project no sophisticated animation are required, so adding buffer support is not a priority ATM. But I'll definitely look into this. Same is true for UART support.

sebi2k1 · 2016-05-16T14:55:55Z

I integrated the requested changes and optimised the timing (one byte takes now 36us). To avoid long periods of disabled IRQs, IRQs are no longer locked for the complete frame but only for a single byte. Tested and verified here.

I cannot proceed with the UART support, since on my platform none of the TXDs is usable (GPIO2/GPIO15).

marcelstoer · 2016-05-16T15:18:12Z

mkdocs.yml

@@ -74,5 +74,6 @@ pages:
        - 'wifi': 'en/modules/wifi.md'
        - 'ws2801': 'en/modules/ws2801.md'
        - 'ws2812': 'en/modules/ws2812.md'
+        - 'tm1829': 'en/modules/tm1829.md'


Sorry for nagging but you should really study the contributing guidelines I pointed you to. The module list here is alphabetically sorted as well.

Got ya. Was reading CONTRIBUTING.md from master branch initially and missed the whole docs/module stuff in dev branch.

…o dev

marcelstoer · 2016-05-16T20:31:45Z

app/include/user_modules.h

@@ -58,6 +58,7 @@
 #define LUA_USE_MODULES_WIFI
 //#define LUA_USE_MODULES_WS2801
 //#define LUA_USE_MODULES_WS2812
+//#define LUA_USE_MODULES_TM1829


Should be on line 53...

Fixed as well.

sebi2k1 · 2016-05-22T08:34:47Z

Are there any additional changes I have to made?

Alkorin · 2016-05-22T18:22:52Z

I'm not a big fan to loose memory and time to invert RGB <=> BRG values. It could be handled in LUA's side. Or directly handled by the developper as I did in the WS2812 driver. Anyway the memcpy is useless you could read in the input buffer and write in your new buffer.

Or as you expose only the "RGB" call, you could invert the values during streaming.

sebi2k1 · 2016-05-22T18:39:50Z

Changing the order in C is likely more efficient than doing it in an interpreted language like LUA, but you are right the memcpy is useless it is just a leftover of the ws2801 driver I used to base my tm1829 support on. I will change that.

Are there any other changes you want me to do in order merge the pull request or do you not consider merging this pull request at all (e.g. cause of bitbanging)?

marcelstoer · 2016-05-22T18:52:22Z

Are there any other changes you want me to do in order merge the pull request or do you not consider merging this pull request at all

I can't test this and I don't understand enough of the technical foundation (C, hardware). Once @Alkorin and @devsaurus are happy with the implementation (as they mainly drove #1263) and see merit in having this module I'd take that as a 👍 for merging. However, that should be done only after the upcoming dev-to-master snap.

devsaurus · 2016-05-22T19:40:52Z

PR looks ok and the input from @Alkorin should be considered. Although I can't check on real hardware, it's also a 👍 from me for a merge after updating master.

Alkorin · 2016-05-22T20:27:08Z

I've checked the datasheets, it seems good.
From my point of vue, RGB is just a convention. I would prefer that the developper writes
tm1829.write(5, string.char(B,R,G,B,R,G)) and avoid malloc / copy in C code

Just said in your API that write() send bytes as-is and it's the developper responsability to put bytes in the correct order. Your repsonsability is just to avoid 0xFF on the first byte to use PWM mode.

Alkorin · 2016-05-22T20:28:39Z

docs/en/modules/tm1829.md

+
+#### Example
+```lua
+tm1829.write(5, string.char(255,0,0,255,0,0) -- turn the two first RGB leds to red using GPIO 5


Missing ) to close the write()

sebi2k1 · 2016-05-24T05:35:31Z

Ok, will do the requested changes.

…o dev

- Stop fighting against the SDK in terms of owning/writing the init_data block. NodeMCU included a default init_data block because originally the SDK did not, but by now it's not needed. - Expose a way to reconfigure the ADC mode from Lua land. With most people using the cloud builder and not able to change the #define for byte 107 this has been a pain point. - Less confusion about which init_data has been used. Lua code can now simply state what mode it wants the ADC to be in, and not worry about the rest of the init_data complexities such as the init_data changing location due to flashing with wrong flash_size setting, or doing/not doing a chip-erase before loading new NodeMCU firmware.

…emcu#1305) Thank you.

…mcu#1307) Chain unhandled load_store exceptions to SDK exception handler

Moved call from `luaopen_wifi (wifi.c)` to `user_rf_pre_init (user_main.c)` Note: This change adds ~440 us to boot time

sebi2k1 · 2016-05-31T18:32:37Z

Let me reopen a clean PR.

Added support for tm1829 based led strips

01f0e9b

marcelstoer reviewed May 16, 2016
View reviewed changes

sebi2k1 added 3 commits May 16, 2016 15:24

Added tm1829 to module docs

3a92b87

Disabled TM1829 by default

74cbf68

Optimised timing and lock IRQs per byte

c1f96cc

marcelstoer reviewed May 16, 2016
View reviewed changes

sebi2k1 added 5 commits May 16, 2016 19:26

Added support for tm1829 based led controllers

38833cb

Merge branch 'dev' of https://github.com/sebi2k1/nodemcu-firmware int…

290a8c7

…o dev

Added support for tm1829 based led controllers

130df25

Merge branch 'dev' of https://github.com/sebi2k1/nodemcu-firmware int…

079cfc6

…o dev

Undo merge

a9f7321

marcelstoer reviewed May 16, 2016
View reviewed changes

Fixed module order

e0fc6cc

Alkorin reviewed May 22, 2016
View reviewed changes

sebi2k1 and others added 4 commits May 31, 2016 20:09

Merge branch 'dev' of https://github.com/sebi2k1/nodemcu-firmware int…

8032aff

…o dev

tm1829: send payload in native format

c588495

Small syntax fix in UART documentation

8d4f94c

marcelstoer and others added 10 commits May 31, 2016 20:25

Add small clarification for 'system restart'

23244ae

Initialize cb for unsubscribe and unref it upon socket deletion. (nod…

e9401f8

…emcu#1305) Thank you.

Remove deploy section (nodemcu#1300)

91ea9f2

Correct socket:send example, fixes nodemcu#1303

4e57d1f

clarify esptool flash options

d8c0300

Add link to esptool flash modes documentation

a020666

Ensure that load_store exceptions trigger the exception handler (node…

98de7e5

…mcu#1307) Chain unhandled load_store exceptions to SDK exception handler

Remove app/json. (nodemcu#1312)

060a1ca

Moved call to set default wifi hostname (nodemcu#1317)

d540114

Moved call from `luaopen_wifi (wifi.c)` to `user_rf_pre_init (user_main.c)` Note: This change adds ~440 us to boot time

Delete http_server.lua, fixes nodemcu#1196

467958d

sebi2k1 closed this May 31, 2016

sebi2k1 deleted the dev branch May 31, 2016 18:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for tm1829 based led strips #1297

Added support for tm1829 based led strips #1297

sebi2k1 commented May 16, 2016

devsaurus commented May 16, 2016

marcelstoer May 16, 2016

marcelstoer commented May 16, 2016

marcelstoer commented May 16, 2016

sebi2k1 commented May 16, 2016 •

edited

Loading

sebi2k1 commented May 16, 2016

marcelstoer May 16, 2016

sebi2k1 May 16, 2016

marcelstoer May 16, 2016

sebi2k1 May 17, 2016

sebi2k1 commented May 22, 2016

Alkorin commented May 22, 2016

sebi2k1 commented May 22, 2016

marcelstoer commented May 22, 2016

devsaurus commented May 22, 2016

Alkorin commented May 22, 2016

Alkorin May 22, 2016 •

edited

Loading

sebi2k1 commented May 24, 2016

sebi2k1 commented May 31, 2016

Added support for tm1829 based led strips #1297

Added support for tm1829 based led strips #1297

Conversation

sebi2k1 commented May 16, 2016

devsaurus commented May 16, 2016

Locking interrupts vs stability

Lua API

marcelstoer May 16, 2016

Choose a reason for hiding this comment

marcelstoer commented May 16, 2016

marcelstoer commented May 16, 2016

sebi2k1 commented May 16, 2016 • edited Loading

sebi2k1 commented May 16, 2016

marcelstoer May 16, 2016

Choose a reason for hiding this comment

sebi2k1 May 16, 2016

Choose a reason for hiding this comment

marcelstoer May 16, 2016

Choose a reason for hiding this comment

sebi2k1 May 17, 2016

Choose a reason for hiding this comment

sebi2k1 commented May 22, 2016

Alkorin commented May 22, 2016

sebi2k1 commented May 22, 2016

marcelstoer commented May 22, 2016

devsaurus commented May 22, 2016

Alkorin commented May 22, 2016

Alkorin May 22, 2016 • edited Loading

Choose a reason for hiding this comment

sebi2k1 commented May 24, 2016

sebi2k1 commented May 31, 2016

sebi2k1 commented May 16, 2016 •

edited

Loading

Alkorin May 22, 2016 •

edited

Loading