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

remapFn for non-rectangles fails *CONTAINS FIX* #9

Closed
jrork opened this issue Mar 16, 2021 · 12 comments
Closed

remapFn for non-rectangles fails *CONTAINS FIX* #9

jrork opened this issue Mar 16, 2021 · 12 comments

Comments

@jrork
Copy link

jrork commented Mar 16, 2021

Thank you for opening an issue on an Adafruit Arduino library repository. To
improve the speed of resolution please review the following guidelines and
common troubleshooting steps below before creating the issue:

  • Do not use GitHub issues for troubleshooting projects and issues. Instead use
    the forums at http://forums.adafruit.com to ask questions and troubleshoot why
    something isn't working as expected. In many cases the problem is a common issue
    that you will more quickly receive help from the forum community. GitHub issues
    are meant for known defects in the code. If you don't know if there is a defect
    in the code then start with troubleshooting on the forum first.

  • If following a tutorial or guide be sure you didn't miss a step. Carefully
    check all of the steps and commands to run have been followed. Consult the
    forum if you're unsure or have questions about steps in a guide/tutorial.

  • For Arduino projects check these very common issues to ensure they don't apply:

    • For uploading sketches or communicating with the board make sure you're using
      a USB data cable and not a USB charge-only cable. It is sometimes
      very hard to tell the difference between a data and charge cable! Try using the
      cable with other devices or swapping to another cable to confirm it is not
      the problem.

    • Be sure you are supplying adequate power to the board. Check the specs of
      your board and plug in an external power supply. In many cases just
      plugging a board into your computer is not enough to power it and other
      peripherals.

    • Double check all soldering joints and connections. Flakey connections
      cause many mysterious problems. See the guide to excellent soldering for examples of good solder joints.

    • Ensure you are using an official Arduino or Adafruit board. We can't
      guarantee a clone board will have the same functionality and work as expected
      with this code and don't support them.

If you're sure this issue is a defect in the code and checked the steps above
please fill in the following fields to provide enough troubleshooting information.
You may delete the guideline and text above to just leave the following details:

  • Arduino board: INSERT ARDUINO BOARD NAME/TYPE HERE

  • Arduino IDE version (found in Arduino -> About Arduino menu): INSERT ARDUINO
    VERSION HERE

  • List the steps to reproduce the problem below (if possible attach a sketch or
    copy the sketch code in too): LIST REPRO STEPS BELOW

I'm using a remap return function for my non-rectangular shaped matrix such as this:
uint16_t pixelArray[12][12] = {0,1,2,3,4,5,6,7,8,9,10,11,
23,22,21,20,19,18,17,16,15,14,13,12,
24,25,26,27,28,29,30,31,32,33,34,35,
47,46,45,44,43,42,41,-1,-1,-1,-1,-1,
48,49,50,51,52,53,54,-1,-1,-1,-1,-1,
71,70,69,68,67,66,65,-1,-1,-1,-1,-1,
72,73,74,75,76,77,78,-1,-1,-1,-1,-1,
95,94,93,92,91,90,89,-1,-1,-1,-1,-1,
96,97,98,99,100,101,102,-1,-1,-1,-1,-1,
119,118,117,116,115,114,113,112,111,110,109,108,
120,121,122,123,124,125,126,127,128,129,130,131,
143,142,141,140,139,138,137,136,135,134,133,132};

uint16_t myRemapFn(uint16_t x, uint16_t y) {
return pixelArray[x][y];
}

Works with Neopixel code because when drawPixel calls setPixelColor, setPixelColor checks if n < numLeds. Framebuffer_GFX just checks to see if x and y are in bounds, but doesn't allow for an odd shape such as mine, so drawPixel simply sets _fb[XY(x,y)] = color. Adding a check to ensure result of XY(x,y) < numpix fixes it.

@marcmerlin
Copy link
Owner

marcmerlin commented Mar 16, 2021

Interesting. normally when I've seen such code, no one writes in the cells that aren't mapped to any hardware pixels, so it's not an issue (you can assign them to 0 instead of -1)

So basically you recommend that I edit
Framebuffer_GFX::drawPixel
and have this instead
uint32_t idx = _fb[XY(x,y)];
if (idx<0 || idx>numpix) return;
_fb[idx] = passThruFlag ? passThruColor : expandColor(color);

I'm not sure how I feel about returning fake index values, when you could just return 0 on empty cells, or numpix-1, or have a empty pixel (define numpix+1 and the last pixel isn't actually there, but exists in the array), but I guess it doesn't hurt to have defense in depth and reject bad mappings either.

If you'd rather keep using -1, can you check the code I just gave you and confirm it fixes your issue?

@jrork
Copy link
Author

jrork commented Mar 16, 2021

uint32_t idx = XY(x,y);
if (idx<0 || idx>numpix) return;
_fb[idx] = passThruFlag ? passThruColor : expandColor(color);

--and--

uint32_t idx = XY(x,y);
if (idx<0 || idx>numpix) return;
_fb[idx] = color;

...works. The code above still crashes as it returns the value of _fb[XY(x,y)] instead of XY(x,y);

0 and numpix-1 are both visible pixels. I've tried the led(numpix+1) trick but it can become a hassle to maintain when setting up the matrix numbering. As you noted, this is a very unusual use case and was difficult to find a working example to pattern off of. However, the examples I did find used -1, and it also makes it much easier to visualize the layout when looking at the array initialization.

Thanks Marc, and thank you for the great libraries! I'll do a post when I'm complete

@marcmerlin
Copy link
Owner

Ok, thanks for confirming. I'll apply #1 then.

@jrork
Copy link
Author

jrork commented Mar 17, 2021

For clarity, i was attempting (and failing) to point out that the fix needed to go into two different places...

marcmerlin added a commit to marcmerlin/Framebuffer_GFX that referenced this issue Mar 17, 2021
@marcmerlin
Copy link
Owner

Ooops, sorry. This should be better.
Next time, feel free to send a PR so that you can get credit for your change :)

@jrork
Copy link
Author

jrork commented Mar 17, 2021

This is my first ever, so didn't realize I could/should do a PR. Next time...

Thanks again!

@marcmerlin
Copy link
Owner

  1. could => you always can :)
  2. should => there is no rule, any contribution is welcome, even if it's a bug report without a fix :)

Basically you can contribute to whichever level is interesting and/or challenging to you, and if you send a PR, you get your name in the git history :)
After some amount of that, you may end up writing your own library and share it with others too.

@jrork
Copy link
Author

jrork commented Mar 17, 2021 via email

@marcmerlin
Copy link
Owner

the fix is already in, so no need to send it again (or send stuff that doesn't really need to be in), but if you find another bug or you want to add a worthwhile addition (and that's more tricky, because I get to decide whether it makes sense to add, or not :) ), feel free

@jrork
Copy link
Author

jrork commented Mar 17, 2021 via email

@marcmerlin
Copy link
Owner

but in the meantime, you can send me details on your project when it's up and running :)

@marcmerlin
Copy link
Owner

look in the history of this issue for
marcmerlin closed this in marcmerlin/Framebuffer_GFX@e14e11a 3 hours ago
marcmerlin added a commit to marcmerlin/Framebuffer_GFX that referenced this issue 2 hours ago
and click on both

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants