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

CPixelView doesn't handle reversed direction and color utility functions #481

Open
MFornander opened this Issue Aug 21, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@MFornander

MFornander commented Aug 21, 2017

We just discovered a few bugs in the rainbow and gradient methods inside CPixelView when the view is mapped on a reverse direction led array. We fixed the rainbow but I'm filing this issue to remember to fix all methods and perhaps even add a few perlin noise methods on CPixelView.

Repro steps

  1. Fold one physical strip of 100 LEDs in half and connect to driver.
  2. Add all 100 LEDs using addLeds.
  3. Create two "logical" view of LEDs using CPixelView's ctor, one with (leds, 0,49) straight and the other as (leds, 99,50) reversed direction.
  4. Call fill_rainbow twice on both views with the same values.

Expected result:
Both logical strips showing the same rainbow, with identical colors side by side, as if they were two different strips connected in parallel to controller.

Actual result:
Incorrectly mapped rainbow color on the reverse direction view.

I'll file a Merge Request with fixes sometime after the dust settles this year.

@MFornander

This comment has been minimized.

Show comment
Hide comment
@MFornander

MFornander Aug 5, 2018

Note to self since we had to recreate the fix again today:

pixelset.h @ 166
OLD AND BUSTED
::fill_rainbow(leds+len+1,-len,initialhue,deltahue);

NEW HOTNESS
::fill_rainbow(leds+len+1,-len,initialhue-deltahue*len,-deltahue);

Still planning to file that MR but want to look at the sibling gradient methods to make sure they are not flawed as well.

MFornander commented Aug 5, 2018

Note to self since we had to recreate the fix again today:

pixelset.h @ 166
OLD AND BUSTED
::fill_rainbow(leds+len+1,-len,initialhue,deltahue);

NEW HOTNESS
::fill_rainbow(leds+len+1,-len,initialhue-deltahue*len,-deltahue);

Still planning to file that MR but want to look at the sibling gradient methods to make sure they are not flawed as well.

@focalintent

This comment has been minimized.

Show comment
Hide comment
@focalintent

focalintent Aug 6, 2018

Member

The reverse direction stuff is something I’m not really happy with in general, and I’m likely to gut it sooner rather than later :)

Member

focalintent commented Aug 6, 2018

The reverse direction stuff is something I’m not really happy with in general, and I’m likely to gut it sooner rather than later :)

@MFornander

This comment has been minimized.

Show comment
Hide comment
@MFornander

MFornander Aug 6, 2018

The reverse direction was absolutely awesome for us. We have two strands folded on a pole, and thus have four CPixelViews as if we had four physical strands. This saved us connectors and development time since other incarnations are not folded. Very cool abstraction. Don't gut it :)

https://www.facebook.com/groups/717748685076630/permalink/769367623248069/

MFornander commented Aug 6, 2018

The reverse direction was absolutely awesome for us. We have two strands folded on a pole, and thus have four CPixelViews as if we had four physical strands. This saved us connectors and development time since other incarnations are not folded. Very cool abstraction. Don't gut it :)

https://www.facebook.com/groups/717748685076630/permalink/769367623248069/

@focalintent

This comment has been minimized.

Show comment
Hide comment
@focalintent

focalintent Aug 6, 2018

Member

Sorry - should've been clearer - I'm going to gut the implementation - I've been meaning to look at some different ways of doing range selectors as a general idea, to give something a lot more flexible (forward ranges, backward ranges, repeat patterns, skip lists, etc...)

Member

focalintent commented Aug 6, 2018

Sorry - should've been clearer - I'm going to gut the implementation - I've been meaning to look at some different ways of doing range selectors as a general idea, to give something a lot more flexible (forward ranges, backward ranges, repeat patterns, skip lists, etc...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment