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.

Sign up for GitHub

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

Jump to bottom

Update Raindrops effect to respect LED range limits #24531

Merged
merged 2 commits into from
Jan 2, 2025
Merged

Update Raindrops effect to respect LED range limits #24531

merged 2 commits into from
Jan 2, 2025

Conversation

filterpaper
Copy link
Contributor

@filterpaper filterpaper commented Oct 26, 2024 •
edited
Loading

Description

Improved effect with update to LED within led_min and led_max limits

Types of Changes

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

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).

@github-actions github-actions bot added the core label Oct 26, 2024
@filterpaper filterpaper force-pushed the raindrops-ledlimits branch 4 times, most recently from 2b3e7c2 to 3e264f3 Compare October 27, 2024 04:34
@filterpaper filterpaper marked this pull request as draft October 27, 2024 04:45
@filterpaper filterpaper marked this pull request as ready for review October 27, 2024 04:48
sigprof
sigprof requested changes Oct 27, 2024
View reviewed changes
quantum/rgb_matrix/animations/raindrops_anim.h Outdated Show resolved Hide resolved
quantum/rgb_matrix/animations/raindrops_anim.h Outdated Show resolved Hide resolved
@filterpaper
Copy link
Contributor Author

Actually you may even keep the old raindrops_set_color() implementation — the only important thing is to call it on the appropriate rendering iteration, so that the LED index is within the range which should be updated

The slight challenge is to avoid updating the LED again until idx has a new random value. The rgb variable is avoided with now by setting idx out of range after an update. Let me know if there's a better suggestion.

@filterpaper filterpaper force-pushed the raindrops-ledlimits branch 4 times, most recently from c1fd315 to 105e83b Compare October 27, 2024 23:43
@drashna drashna requested a review from a team October 28, 2024 00:34
@filterpaper filterpaper requested a review from sigprof October 28, 2024 00:44
@github-actions GitHub Actions
Copy link

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 bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Dec 12, 2024
@filterpaper
Update Raindrops effect to respect LED range limits
1596033 
Improved effect to update LED index within `led_min` and `led_max` limits
@filterpaper
Remove static RGB variable
2f07445 
Set index out of range after trigger
@filterpaper
Copy link
Contributor Author

Pull request is still awaiting review.

@github-actions github-actions bot removed the stale Issues or pull requests that have become inactive without resolution. label Dec 13, 2024
@drashna drashna added awaiting review breaking_change_2025q1 Targeting breaking changes Q1 2025 labels Dec 22, 2024
@drashna drashna merged commit e016b9b into qmk:develop Jan 2, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tzarc tzarc tzarc approved these changes

@drashna drashna drashna approved these changes

@sigprof sigprof Awaiting requested review from sigprof

Assignees
No one assigned
Labels
awaiting review breaking_change_2025q1 Targeting breaking changes Q1 2025 core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@filterpaper @sigprof @drashna @tzarc