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

Fix Meridian RGB indicators for varying hardware across PCB batches. #11812

Closed
wants to merge 13 commits into from
Closed

Fix Meridian RGB indicators for varying hardware across PCB batches. #11812

wants to merge 13 commits into from

Conversation

holtenc
Copy link
Contributor

@holtenc holtenc commented Feb 6, 2021

Two different LED parts were used by the manufacturer creating a firmware issue. This update helps to correct this by making two different firmware versions. A small change was made to drivers/chibios/ws2812.c to make this possible.

WS2812 driver was also changed from SPI to bitbang due to an apparent issue with SPI introduced since the Meridian firmware was initially merged.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

#11755

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • 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).

drivers/chibios/ws2812.c Outdated Show resolved Hide resolved
keyboards/primekb/meridian/v1.1/readme.md Outdated Show resolved Hide resolved
keyboards/primekb/meridian/v1/readme.md Show resolved Hide resolved
keyboards/primekb/meridian/meridian.c Show resolved Hide resolved
keyboards/primekb/meridian/meridian.c Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the core label Feb 7, 2021
Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also v1.1 is currently an invalid keyboard name so would have to be something like v1_1

keyboards/primekb/meridian/v1.1/config.h Outdated Show resolved Hide resolved
keyboards/primekb/meridian/v1.1/rules.mk Outdated Show resolved Hide resolved
keyboards/primekb/meridian/v1/config.h Show resolved Hide resolved
keyboards/primekb/meridian/v1/rules.mk Outdated Show resolved Hide resolved
@holtenc holtenc requested review from zvecr and drashna February 7, 2021 22:08
@holtenc
Copy link
Contributor Author

holtenc commented Feb 7, 2021

build errors in continuous-integration/travis-ci/pr are due to my changes in drivers/chibios/ws2812.c not being present yet.

Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also as a passing comment, the rev1 now using the bitbang driver seems like a step backwards. And use of rgblight layers at the keyboard level seems a tad meh.

keyboards/primekb/meridian/rules.mk Outdated Show resolved Hide resolved
@holtenc holtenc requested a review from zvecr February 10, 2021 04:06
@holtenc
Copy link
Contributor Author

holtenc commented Feb 19, 2021

@drashna @zvecr
Do you guys have any further revisions that need to be made? I can get that PR for /drivers/chibios/ws2812.c submitted as soon as this side is taken care of.

keyboards/primekb/meridian/meridian.c Outdated Show resolved Hide resolved
keyboards/primekb/meridian/meridian.c Show resolved Hide resolved
@holtenc holtenc requested a review from drashna February 22, 2021 15:33
@holtenc
Copy link
Contributor Author

holtenc commented Apr 15, 2021

So would it be best to just close this PR and re-submit? @zvecr @drashna

from what I can tell the structure of halconf.h and chconf.h etc have had significant changes, which I attempted to update to but that just seems to have messed this PR up.

please advise.

@drashna
Copy link
Member

drashna commented Apr 19, 2021

Well, updating them should be just running qmk chibosconf, I think it was.

But you roll back commits and force push, or revert them, to fix issues.

@stale
Copy link

stale bot commented Jun 3, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@stale
Copy link

stale bot commented Jul 8, 2021

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.

@stale stale bot closed this Jul 8, 2021
sigprof added a commit to sigprof/qmk_firmware that referenced this pull request Oct 4, 2021
Use TIM1_CH3N instead of TIM15_CH2, because TIM15 on F072 is apparently
not supported properly in ChibiOS 20.3.3.

Also try to adjust some WS2812 timings as suggested in qmk#11812 (but not
T1H yet - that would also require changes in the WS2812 PWM driver).
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.

4 participants