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

[Bug]: Both Ends progress effect skips the last LED if the strip has an odd number of LEDs #209

Open
CoryCharlton opened this issue Jan 27, 2023 · 5 comments
Labels
improvement Improvement to existing functionality solved Solved, but not yet released.
Milestone

Comments

@CoryCharlton
Copy link

Description of the bug

The Both Ends progress effect skips the last LED if the strip has an odd number of LEDs.

My test strip is weird because I borrowed it from a nightlight I'm building for my daughter while checking out the plugin. In these screenshots LED 1 is in the upper right and LED 63 is in the bottom left and each row alternates data direction.

Like this:

<-  9  8  7  6  5  4  3  2  1
-> 10 11 12 13 14 15 16 17 18
<- 27 26 25 24 23 22 21 20 19
-> 28 29 30 31 32 33 34 35 36
<- 45 44 43 42 41 40 39 38 37
-> 46 47 48 49 50 51 52 53 54
<- 63 62 61 60 59 58 57 56 55

These screenshots are of the Printing Progess set to Both Ends and you can see that LED 1 is lit but LED 63 is not.

20230126_213456

20230126_213512

I toggled torch mode just to show that the LED is working correctly.

20230126_233002

Steps to reproduce

  1. Configure your strip to an odd number of LEDs (even if your strip is even)
  2. Configure a status for Both Ends
  3. Get the printer to that state and confirm the "last" LED is not lit

What happened instead?

The last LED does not light and progress begins from the last even numbered LED

Plugin version

0.8.1

OctoPrint version

1.8.6

(if relevant) OctoPi version

1.0.0 RC3

Log Files & Screenshots

No response

Anything else?

No response

@github-actions github-actions bot added the potential bug Bug reported but yet to be analysed label Jan 27, 2023
@CoryCharlton
Copy link
Author

CoryCharlton commented Jan 27, 2023

Started looking at the code and I saw this commit which is the root cause. It appears to be the intended behavior at this point.

That said I'll look at the code more to see if I can send over a PR to improve this. At first glance I think the correct solution for an odd numbered strip is to start from both ends and "share" the left over LED between both sides.

CoryCharlton added a commit to CoryCharlton/OctoPrint-WS281x_LED_Status that referenced this issue Jan 27, 2023
@cp2004
Copy link
Owner

cp2004 commented Jan 29, 2023

It was intended behaviour originally, because I wanted to make the effect symmetrical & using an odd number of LEDs resulted in confusing behaviour where the last few steps of the progress weren't represented properly if I remember right. Thinking about it now (it was a long time ago that I added the effect) the middle LED could be shared on an odd setup without impacting the progress steps.

@CoryCharlton
Copy link
Author

Yup, that's what I did in my PR #211. The middle pixel is shared between both segments. I've been running it through multiple prints since I submitted the PR and it's working great.

@cp2004
Copy link
Owner

cp2004 commented Jan 29, 2023

I'll play with that once I am back with my development setup later this week - thank you for the PR!

@CoryCharlton
Copy link
Author

Solution is implemented in PR #212

cp2004 added a commit that referenced this issue Jan 29, 2023
* Fixes issue #210 by adding the `reverse` parameter and setting the left/right `reverse_progress` parameter based on that

* Refactored progress bar effect into a shared method

* Refactored both ends effect to use the shared progress_bar_impl method

* Added the "From Center" progress effect

* Added the "Reversed Progress Bar" progress effect

* Remove `Reverse progress bar direction` from the UI and from being passed into the `EffectRunner`

* ↔️ Add settings migration

* 🎨 reverse is already a bool

Closes #209 
Closes #210

---------

Co-authored-by: Charlie Powell <[email protected]>
@cp2004 cp2004 added solved Solved, but not yet released. improvement Improvement to existing functionality and removed potential bug Bug reported but yet to be analysed labels Jan 29, 2023
@cp2004 cp2004 added this to the 1.0.0 milestone Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement to existing functionality solved Solved, but not yet released.
Projects
None yet
Development

No branches or pull requests

2 participants