NeoPixelBus icon indicating copy to clipboard operation
NeoPixelBus copied to clipboard

large pixel count make atmega 32u4 not working proprely

Open dsimonet opened this issue 4 years ago • 7 comments

Describe the bug When using a large amount of pixel (999) with : NeoPixelBus<NeoGrbFeature, Neo800KbpsMethod> strip(PixelCount, PixelPin); it become impossible to reprogram arduino proprely. Due the very specific uart communication of the 32u4 (micro and leonardo)

To Reproduce Steps to reproduce the behavior:

  1. take any arduino with 32u4 mcu
  2. create a large very huge pixel count (it crash with 999, maybe before)
  3. Push your code
  4. You will not be able to find your arduino in the port menu

to recover, you have to reset your mcu with button or reset pin short to gnd. Then you have a few second to select it in the menu and program it.

dsimonet avatar May 20 '20 06:05 dsimonet

You are running out of memory. When you compile, it should output how much RAM is left. Compare that to the dynamic allocation needed to support that many pixels.

998 (pixels) x 3 (rgb) = 2994 bytes of ram for the pixel buffer.

It only has 2K ram. That chip can't support that many and it seems it behaves poorly if you over alloc memory.

You could reproduce the problem by this simple line of code placed within setup()

uint8_t* bigbuf = new uint8_t[2994];

There is no memory check before allocation; this is left up to you make sure it won't over allocate. This is due to low program space of these chips and the need for the library to run on very low memory versions.

Makuna avatar May 20 '20 07:05 Makuna

Thanks for your fast reply. Yes, I guess this was a bad memory allocation. In fact, I lost 1 hours to get out of the situation, so I wanted to share my experience.

May you can put an information somewhere to advise of this behavior. This is a common issue shared by all mcu platform but specially with 32u4 this small issue can be very tricky.

But your are right, this can append from any bad malloc, it's not relative to your library. Which is perfect, by the way, for control of exotic LED.

dsimonet avatar May 20 '20 07:05 dsimonet

reopened to track improving wiki on the issue of memory usage.

Makuna avatar May 20 '20 07:05 Makuna

;-)

dsimonet avatar May 20 '20 08:05 dsimonet

This is not necessarily a memory allocation problem.

There is some funky logic going on when entering the bootloader: This working properly depends on the content of a memory address in the middle of the adressable space.

I've debugged this for ages until I finally managed to run two strips with 300 LEDs each.

The (rather ugly and invasive - cli()'s for very long stretches) workaround is in https://github.com/simon-budig/flame - but I'll cut'n'paste the relevant code here for posterity.

void
loop ()
{
  [... variable declarations, not a lot of running code ...]

#ifdef MAGIC_KEY_POS
  // for atmega32u4 based Arduinos:
  //
  // check if the bootloader has been activated.
  // avoid doing any rendering to prevent the
  // MAGIC_KEY getting overridden which in turn
  // would prevent entering the bootloader properly.

  if (*((uint16_t *) MAGIC_KEY_POS) == MAGIC_KEY &&
      WDTCSR & (1 << WDE))
    {
      return;
    }
#endif

[...]

#ifdef MAGIC_KEY_POS
  // Now, this is quite unfortunate:
  //
  // for the atmege32u4 based arduinos (Leonardo, pro micro etc.)
  // entering the bootloader is initiated in the USB interrupt
  // handler (i.e. can happen at any time).
  //
  // This does two things: writes MAGIC_KEY to MAGIC_KEY_POS and
  // enables the watchdog reset.
  //
  // If the watchdog fires the atmega32u4 resets and the bootloader
  // code checks for the MAGIC_KEY at MAGIC_KEY_POS. If it finds
  // the MAGIC_KEY it sticks in the bootloader mode.
  //
  // for larger LED strips it is quite likely that MAGIC_KEY_POS
  // resides in the middle of the framebuffer. And if the USB interrupt
  // happens while the code is rendering stuff to the framebuffer,
  // it then might happen that the MAGIC_KEY immediately gets overwritten
  // by the rendering code. This prevents that the bootloader gets
  // entered upon the watchdog reset. For some effects the AVR is mostly
  // rendering, making it basically impossible to enter the bootloader
  // via the IDE.
  //
  // As a workaround we disable all interrupts during the rendering code
  // which is quite a brute force method. This delays the writing of the
  // MAGIC_KEY to the point of the sei() (since this is now the point where
  // the USB interrupt gets handled), giving the MAGIC_KEY precedence over
  // the rendered effect.
  //
  // and since we basically avoid running loop() when the
  // bootloader-conditions are met (see above) the switch to the bootloader
  // now is more reliable again.

  cli ();
#endif

[... code writing into the framebuffer ...]

#ifdef MAGIC_KEY_POS
  sei ();
#endif

simon-budig avatar Apr 09 '21 08:04 simon-budig

@simon-budig Your comment here scares me... " // for larger LED strips it is quite likely that MAGIC_KEY_POS // resides in the middle of the framebuffer. And if the USB interrupt // happens while the code is rendering stuff to the framebuffer, // it then might happen that the MAGIC_KEY immediately gets overwritten // by the rendering code. This prevents that the bootloader gets "

If this is true, then this is truly a bug in the Arduino core, the MAGIC_KEY_POS should never reside in memory that can be used by a sketch, either allocated or on the stack. If it can, then ANY SKETCH could run into a problem.

You should raise this issue with the Arduino team.

UPDATE: I checked the Arduino code, this is only an issue for older bootloader versions (~pre 2014), newer ones are supposed to use the last two bytes of the RAM address space; which is the stack and thus the return value from the internally hidden application main() so it isn't a problem. This merge fixed it back in 2016. Well, not a problem as long as you don't write over the end of memory; like what I suspect happened above when you try to use more pixels than you have of memory. I believe I have an open issue to have "Begin()" return an error if allocation fails. Another thing that might help is when the data is being copied, the end "test" includes a check for the end of RAM - 2 to also avoid overwriting this MAGIC_KEY for newer bootloaders.

Makuna avatar Apr 09 '21 17:04 Makuna

It is perfectly possible that there has been an update to the bootloader. Not sure though if it really gets shipped with all these atmega32u4 based arduino pro micro clones... :-)

Yeah, if you're able to update the bootloader with an ISP programmer then this is the most sane solution.

simon-budig avatar Apr 15 '21 19:04 simon-budig