Why did this code stop working?

Using Arduino Programming Questions
#1

I wrote a simple sketch to output 8 random digits at the press of a button on a Leonardo-class Arduino. It’s worked fine for a couple of weeks but today it stopped working.

After hours of trying things I’ve found where the problem is - and worked around it - but I’m still at a complete loss to understand what changed.

In a nutshell - this code doesn’t work anymore:

for (uint8_t loopCount; loopCount < randomNumberLength; loopCount++) 
    {
      Keyboard.print(random(0, 10)); 
    }

Whereas this slightly longer work-around works just fine:

loopCount = 0;

while (loopCount < randomNumberLength) 
  {
    Keyboard.print(random(0, 10));
    loopCount++; 
  }

Note: randomNumberLength has been previously defined in both versions and loopCount was previously defined in the 2nd example.

When the problem started occuring the “for” condition was never satisfied so nothing was ever output.

The issue occured when I tried to add an additional control to ensure that the key had been pressed for 200mS prior to continuing (to try to stop some spurious activations that have been occuring). I mucked something up - reverted to a backup (which I’m 99.9999% certain was a backup of the last version of the working code) - and it was all steadily downhill from there.

I eventually traced it to the above sections of code; it’s almost like the compiler has “updated itself” and is now “getting the first example wrong” (despite “check for updates” NOT being selected).

Can anyone shed any light on this? It’s a curveball I really didn’t see coming.

#2

uint8_t is the same as a byte. If randomNumberLength is an int or larger, when it goes above 256, the for() will never be true.

#3

As a general rule, if you think you found a compiler bug, it is your code that is broken. Before a compiler is released into a product like Arduino it has run trough more code than you or me will ever write in our life.

Sometimes compilers can spot issues that are not errors. Make sure to enable compiler warnings.

File → Preferences → Compiler Warnings : All

Sometimes you will get a lot of warnings from libraries and often you can ignore them. Try to look for the warnings in your files.

#4

Yep, but in both examples randomNumberLength is only 8.

#5

I’d agree with that rule 100%; heck if I had a dollar for every time I’d forgotten a semicolon I’d be a millionaire (and that’s just in the past week!).

… but I’m just at a loss to understand (a) why something that did work, now no longer works, and (b) to me (and others?) it looks like a perfectly valid piece of code … it just doesn’t work anymore.

#6

I do not have a Leonardo but this is what the GCC ARM compiler says about the code.

warning: ‘loopCount’ may be used uninitialized in this function [-Wmaybe-uninitialized]

So, if loopCount is larger than randomNumberLength by random chance or because the register used for it has a different value because of changes in the code the for loop will never run.

Simple fix:

for (uint8_t loopCount = 0; loopCount < randomNumberLength; loopCount++)
#7

Thanks Klaus,

That makes a lot of sense. I’ll test it some more; at least at this stage it seems 100% repeatable - so should be easy enough to test now that I know it’s not hardware related.

#8

That was probably obvious in the code you didn’t post.

#9

If anyone’s interested, here’s the complete code:

/*  This program inserts a random 8 digit number at the current cursor position when the switch is pressed.*/

#include "Keyboard.h"                                                                 // Include keyboard control library

// Declare constants

   int const seedPin   = 1;                                                           //
   int const switchPin = 10;                                                          //

// Declare Variables

   uint8_t  seedBitValue       = 0;                                                   //
   uint8_t  seedByteValue      = 0;                                                   //
   uint32_t seedWordValue      = 0;                                                   //
   uint8_t  randomNumberLength = 8;                                                   //
   uint8_t  loopCount          = 0;                                                   //

// SETUP CODE

void setup()                                                                          // Setup code starts here (runs once)
{
  pinMode(switchPin, INPUT);                                                          // Setup port for switch
  Keyboard.begin();                                                                   // Initialise keyboard
  
  for (uint8_t wordShift = 0; wordShift < 4; wordShift++)                             // 4 bytes in a 32 bit word
  {
    for (uint8_t byteShift = 0; byteShift < 8; byteShift++)                           // 8 bits in a byte
    {
        seedBitValue = (analogRead(seedPin) & 0x01);                                  // Flip the coin
        delay(1);                                                                     // Delay a single millisecond to allow the pin to fluctuate
        seedByteValue = seedByteValue | ((seedBitValue & 0x01) << byteShift);         // Build a stack of eight flipped coins
        seedBitValue = 0;                                                             // Clear out the previous coin value
    }
    seedWordValue = seedWordValue | (uint32_t)seedByteValue << (8 * wordShift);       // Build a stack of four sets of 8 coins (shifting right creates a larger number so cast to 32bit)
    seedByteValue = 0;                                                                // Clear out the previous stack value
  }
  randomSeed(seedWordValue);                                                          // Seed the random number generator
}                                                                                     // End of setup code

// MAIN CODE
void loop()
{
  while (digitalRead(switchPin) == 1)                                                 // Is key pressed?
    {
      delay(200);                                                                     // Wait 200mS
      while (digitalRead(switchPin) == 1)                                             // Is key STILL pressed after 200mS if so then proceed
      {
        loopCount = 0;                                                                // Zero loop counter
        while (loopCount < randomNumberLength)                                        // Do it 8 times
          {
            Keyboard.print(random(0, 10));                                            // Output a random number
            loopCount++;                                                              // Inc loop counter
          }
        delay(1000);                                                                  // Wait 1 second before restarting loop to give time for switch release
      }
    }
}                                                                                     // End of Main Code Loop

I must give credit to https://rheingoldheavy.com/ - I leaned heavily on their example for the code to seed the RNG. https://rheingoldheavy.com/better-arduino-random-values/

I’m pleased with the way it works; I use random numbers for invoices - now all I have to do is push a button to have 8 random digits inserted at the current cursor position.

#10

Thanks folks - Klaus hit the nail on the head, and the code is now working as expected.

My mistake was assuming an unititialised variable would be zero; I guess there’s a certain “satorial eloquence” in creating a random number generator that only “randomly works”!

Most importantly of all, I’ve learned something significant today … and for that I am very grateful.

Many thanks to all who responded - especially Klaus; much appreciated.

#11

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.