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

Move Ergodox EZ RGB Light code to custom driver #7309

Merged
merged 7 commits into from
Nov 14, 2019

Conversation

drashna
Copy link
Member

@drashna drashna commented Nov 9, 2019

Description

This pulls out the RGBW_BB_TWI implementation out of the core ws2812 driver, since only the Ergodox EZ actually uses this.

Additionally. this adds support for fully addressing the RGB Strip in the ErgoDox EZ, rather than only just mirroring the halves.

Thanks to @seebs for the original code to implement the addressing, and thanks to @zvecr for help and inspiration

Types of Changes

  • Core
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@seebs
Copy link
Contributor

seebs commented Nov 11, 2019

So, re-reading this: The ergodox EZ has other code talking over i2c also, but it looks like this part is just directly bitbanging the i2c hardware. How does that interact with the i2c code? I suppose it appears to work, so it must not be all THAT bad, but it feels sorta ugly.

quantum/rgblight.c Outdated Show resolved Hide resolved
keyboards/ergodox_ez/ws2812_bb_i2c.c Outdated Show resolved Hide resolved
@drashna
Copy link
Member Author

drashna commented Nov 11, 2019

@seebs I'm not exactly sure what you mean there.

But all this code has already existed and is what is actively being used. It was more of a matter of "where", as all the I2C code for the LEDs was actually in core files, and ONLY the ergodox EZ was using it.

Also, this has some additional work needed, to make things cleaner, especially as I did mean to open this as a draft...

@seebs
Copy link
Contributor

seebs commented Nov 11, 2019

I'm ... actually now very confused and no longer sure what I'm talking about.

So far as I know, the EZ has only one I2C bus, the four-wire cable running from the (AVR) right hand to the (MCP23018) left hand.

Most of the EZ code uses i2c_master.h/i2c_master.c to drive these, using functions with names like i2c_start which use the TWSR and related TWI registers to have the chip's built-in TWI driver do things.

The i2c code that was in ws_2812, and is now in the ergodox directory, is bit-banging pins on PORTD. As it happens, those are the same pins used by the i2c hardware in the chip; PD0 == SCL, PD1 == SDA.

This isn't a criticism of the move, which obviously doesn't change it, but I'm not sure how this interacts with any other use of these pins from code that isn't the ws2812 driving code, since the chip has built-in hardware that can use these pins, and as I understand it, can use them asynchronously from other code running on the chip.

@drashna
Copy link
Member Author

drashna commented Nov 11, 2019

I'm ... actually now very confused and no longer sure what I'm talking about.

I ... 100% understand this, and I've been there more times than I can count!

So far as I know, the EZ has only one I2C bus, the four-wire cable running from the (AVR) right hand to the (MCP23018) left hand.

Correct.

Most of the EZ code uses i2c_master.h/i2c_master.c to drive these, using functions with names like i2c_start which use the TWSR and related TWI registers to have the chip's built-in TWI driver do things.

The i2c code that was in ws_2812, and is now in the ergodox directory, is bit-banging pins on PORTD. As it happens, those are the same pins used by the i2c hardware in the chip; PD0 == SCL, PD1 == SDA.

This isn't a criticism of the move, which obviously doesn't change it, but I'm not sure how this interacts with any other use of these pins from code that isn't the ws2812 driving code, since the chip has built-in hardware that can use these pins, and as I understand it, can use them asynchronously from other code running on the chip.

I can't say that I'm expert here, but .... the code added has been what has been used by the ergodox EZ for quite some time now. The only difference now, is that it's in the ergodox EZ's folder, rather than in the core driver for ws2812. It's functionally identical, with the only change being "where" the code is located.

And you're probably correct, the code could be cleaned up and handled with the standard functions. However, I'll have to see about wrapping my head around that, and testing it. Something that tends to be more challenging for me. But ... I've been expanding my skills, so ....

@drashna
Copy link
Member Author

drashna commented Nov 11, 2019

This should be a much cleaner implementation, and affect a minimal number of core files.

@drashna drashna requested a review from a team November 11, 2019 22:47
@drashna drashna requested review from a team, zvecr and fauxpark November 12, 2019 16:33
@drashna drashna merged commit 1cf63a1 into qmk:master Nov 14, 2019
@drashna drashna deleted the keyboard/ergodox_ez_custom branch November 14, 2019 20:00
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Nov 15, 2019
* 'master' of https://github.com/qmk/qmk_firmware: (475 commits)
  [Keyboard] kbdfans keyboards NKRO enable (qmk#7364)
  [Keyboard] fix DZ60RGB info.json (qmk#7362)
  Adding new pcb with default keymap and personal keymap (qmk#7314)
  [Core] Cleanup rules.mk for F303 keyboards (qmk#7306)
  [Docs] Japanese translation of docs/ja/newbs_best_practices.md (qmk#7337)
  Set device version from config.h for V-USB boards (qmk#7316)
  Add support for configurable polling interval and power usage o… (qmk#7336)
  capslock_led (qmk#7359)
  Move Ergodox EZ RGB Light code to custom driver  (qmk#7309)
  Fix shell.nix by pinning nixpkgs (qmk#6213)
  [Keyboard] add kbdmini; dztech, kbdfans keyboards cleanup (qmk#7223)
  [Docs] Encourage newbs to not download the repo as a zip (qmk#7353)
  Update debounce docs (qmk#7355)
  [Keyboard] Add TG4x (qmk#7351)
  [Keyboard] Add FLX Virgo (qmk#7352)
  format code according to conventions [skip ci]
  Adding verd layout to RSII (qmk#7296)
  Add my custom layouts for GH60, DZ60 and Minivan (qmk#7278)
  [Keyboard] Added abnt2 layout to dz60 (qmk#7340)
  [Keyboard] add Little Keyboards as a seller of helix pcbs outside of japan (qmk#7249)
  ...
drashna added a commit to zsa/qmk_firmware that referenced this pull request Nov 17, 2019
* Move Ergodox EZ RGB code to custom driver

Also implements full addressing of Ergodox EZ's LED Strip, as written by seebs
Co-authored-by: Seebs <[email protected]>

* Make Clipping range accessible for custom drivers

* Remove RGBW_BB_TWI from driver and docs

* Revert changes to clipping range support

* Use just rgblight_set instead of full custom driver

* Convert to i2c_master commands

* Rename rgblight driver and clean up includes
ripxorip pushed a commit to ripxorip/qmk_firmware that referenced this pull request Dec 3, 2019
* Move Ergodox EZ RGB code to custom driver

Also implements full addressing of Ergodox EZ's LED Strip, as written by seebs
Co-authored-by: Seebs <[email protected]>

* Make Clipping range accessible for custom drivers

* Remove RGBW_BB_TWI from driver and docs

* Revert changes to clipping range support

* Use just rgblight_set instead of full custom driver

* Convert to i2c_master commands

* Rename rgblight driver and clean up includes
drashna added a commit to zsa/qmk_firmware that referenced this pull request Dec 6, 2019
* Move Ergodox EZ RGB code to custom driver

Also implements full addressing of Ergodox EZ's LED Strip, as written by seebs
Co-authored-by: Seebs <[email protected]>

* Make Clipping range accessible for custom drivers

* Remove RGBW_BB_TWI from driver and docs

* Revert changes to clipping range support

* Use just rgblight_set instead of full custom driver

* Convert to i2c_master commands

* Rename rgblight driver and clean up includes
patrl pushed a commit to patrl/qmk_firmware that referenced this pull request Dec 29, 2019
* Move Ergodox EZ RGB code to custom driver

Also implements full addressing of Ergodox EZ's LED Strip, as written by seebs
Co-authored-by: Seebs <[email protected]>

* Make Clipping range accessible for custom drivers

* Remove RGBW_BB_TWI from driver and docs

* Revert changes to clipping range support

* Use just rgblight_set instead of full custom driver

* Convert to i2c_master commands

* Rename rgblight driver and clean up includes
drashna added a commit to zsa/qmk_firmware that referenced this pull request Jan 2, 2020
* Move Ergodox EZ RGB code to custom driver

Also implements full addressing of Ergodox EZ's LED Strip, as written by seebs
Co-authored-by: Seebs <[email protected]>

* Make Clipping range accessible for custom drivers

* Remove RGBW_BB_TWI from driver and docs

* Revert changes to clipping range support

* Use just rgblight_set instead of full custom driver

* Convert to i2c_master commands

* Rename rgblight driver and clean up includes
fdidron added a commit to zsa/qmk_firmware that referenced this pull request Jan 6, 2020
* ARM - ws2812 bitbang (qmk#7173)

* Initial ARM bitbang ws2812 driver

* Unify chibios platform to run rgblight_task

* Remove 'avr only' comments from ws2812 docs

* Remove 'avr only' comments from ws2812 docs

* Unify chibios platform to run rgblight_task - review comments

* Remove debug flags from keymap

* Add comments from review

* Add defines for STM32L0XX

* Attempt to get arm ws2812 working on multiple gcc versions

* Support RGBLIGHT_SLEEP when ChibiOS boards suspend (qmk#7280)

Copypasta from the AVR suspend implementation with a Teensy-specific
hack removed

* Unify RGB and RGBW commands (qmk#7297)

* Fix unicode in comments

Co-Authored-By: fauxpark <[email protected]>

* Remove separate RGBW implementation for a unified function

* Set White to 0 in RGBW LEDs

This is just to get this working, later, proper brightness can be handled elsewhere.

* Use us instead of nanoseconds(?) since it renders correctly on web

* Remove RGBW function from arm/ws2812.h

* Remove RGBW function from arm/ws2812.c

* Formatting changes

* Add doc info

* Remove force of debug on within rgblight - causes lockups waiting for hid_listen (qmk#7330)

* Move Ergodox EZ RGB Light code to custom driver  (qmk#7309)

* Move Ergodox EZ RGB code to custom driver

Also implements full addressing of Ergodox EZ's LED Strip, as written by seebs
Co-authored-by: Seebs <[email protected]>

* Make Clipping range accessible for custom drivers

* Remove RGBW_BB_TWI from driver and docs

* Revert changes to clipping range support

* Use just rgblight_set instead of full custom driver

* Convert to i2c_master commands

* Rename rgblight driver and clean up includes

* Use White channel on RGBW LEDs

* SPI DMA based RGB Underglow for STM32 (qmk#7674)

* Initial stash of ws2812 spi driver

* Update comment, add sync backup plan

* Add testing notes to spi ws2812 driver

* Align RGBW error messages

Co-authored-by: Joel Challis <[email protected]>
Co-authored-by: Jonathan Rascher <[email protected]>
Co-authored-by: Florian Didron <[email protected]>
fdidron added a commit to zsa/qmk_firmware that referenced this pull request Jan 8, 2020
* ARM - ws2812 bitbang (qmk#7173)

* Initial ARM bitbang ws2812 driver

* Unify chibios platform to run rgblight_task

* Remove 'avr only' comments from ws2812 docs

* Remove 'avr only' comments from ws2812 docs

* Unify chibios platform to run rgblight_task - review comments

* Remove debug flags from keymap

* Add comments from review

* Add defines for STM32L0XX

* Attempt to get arm ws2812 working on multiple gcc versions

* Support RGBLIGHT_SLEEP when ChibiOS boards suspend (qmk#7280)

Copypasta from the AVR suspend implementation with a Teensy-specific
hack removed

* Unify RGB and RGBW commands (qmk#7297)

* Fix unicode in comments

Co-Authored-By: fauxpark <[email protected]>

* Remove separate RGBW implementation for a unified function

* Set White to 0 in RGBW LEDs

This is just to get this working, later, proper brightness can be handled elsewhere.

* Use us instead of nanoseconds(?) since it renders correctly on web

* Remove RGBW function from arm/ws2812.h

* Remove RGBW function from arm/ws2812.c

* Formatting changes

* Add doc info

* Remove force of debug on within rgblight - causes lockups waiting for hid_listen (qmk#7330)

* Move Ergodox EZ RGB Light code to custom driver  (qmk#7309)

* Move Ergodox EZ RGB code to custom driver

Also implements full addressing of Ergodox EZ's LED Strip, as written by seebs
Co-authored-by: Seebs <[email protected]>

* Make Clipping range accessible for custom drivers

* Remove RGBW_BB_TWI from driver and docs

* Revert changes to clipping range support

* Use just rgblight_set instead of full custom driver

* Convert to i2c_master commands

* Rename rgblight driver and clean up includes

* Use White channel on RGBW LEDs

* SPI DMA based RGB Underglow for STM32 (qmk#7674)

* Initial stash of ws2812 spi driver

* Update comment, add sync backup plan

* Add testing notes to spi ws2812 driver

* Align RGBW error messages

Co-authored-by: Joel Challis <[email protected]>
Co-authored-by: Jonathan Rascher <[email protected]>
Co-authored-by: Florian Didron <[email protected]>
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
* Move Ergodox EZ RGB code to custom driver

Also implements full addressing of Ergodox EZ's LED Strip, as written by seebs
Co-authored-by: Seebs <[email protected]>

* Make Clipping range accessible for custom drivers

* Remove RGBW_BB_TWI from driver and docs

* Revert changes to clipping range support

* Use just rgblight_set instead of full custom driver

* Convert to i2c_master commands

* Rename rgblight driver and clean up includes
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Move Ergodox EZ RGB code to custom driver

Also implements full addressing of Ergodox EZ's LED Strip, as written by seebs
Co-authored-by: Seebs <[email protected]>

* Make Clipping range accessible for custom drivers

* Remove RGBW_BB_TWI from driver and docs

* Revert changes to clipping range support

* Use just rgblight_set instead of full custom driver

* Convert to i2c_master commands

* Rename rgblight driver and clean up includes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants