Go Down

Topic: Whack-a-bug (Read 1 time) previous topic - next topic

Coding Badly


I just spent about 30 minutes tracking down a very annoying bug.  The code below is the distillation.  Can you spot the problem?  (Code should run on any Arduino compatible.)

Code: [Select]
#define USISIF  7
#define USIOIF  6
#define USIPF   5
#define USIDC   4
#define USICNT3 3
#define USICNT2 2
#define USICNT1 1
#define USICNT0 0

void setup( void )
{
  Serial.begin( 115200 );
}

void loop( void )
{
  Serial.print(
      (1 << USIOIF)
      |  // USIOIF: Counter Overflow Interrupt Flag; cleared because it is used in the loop below.
//      (1 << USIPF) | // USIPF: Stop Condition Flag; cleared for no particular reason.
//      (1 << USIDC) | // USIDC: Data Output Collision; cleared for no particular reason.
      (0 << USICNT3) | (0 << USICNT2) || (0 << USICNT1) | (0 << USICNT0)  // USICNT3:0: Counter Value; cleared because it is used in the loop below.
      , HEX );

  Serial.print( F(" =? " ) );

  Serial.print(
      (1 << USIOIF)
//      |  // USIOIF: Counter Overflow Interrupt Flag; cleared because it is used in the loop below.
//      (1 << USIPF) | // USIPF: Stop Condition Flag; cleared for no particular reason.
//      (1 << USIDC) | // USIDC: Data Output Collision; cleared for no particular reason.
//      (0 << USICNT3) | (0 << USICNT2) || (0 << USICNT1) | (0 << USICNT0)  // USICNT3:0: Counter Value; cleared because it is used in the loop below.
      , HEX );

  Serial.println();

  delay( 1000 );
}

MichaelMeissner

#1
Apr 03, 2013, 02:54 pm Last Edit: Apr 03, 2013, 02:58 pm by MichaelMeissner Reason: 1
You used '||' in one place instead of '|'
Code: [Select]

(0 << USICNT2) || (0 << USICNT1)


Also, you really don't need all of the 0 << n, since that is always 0, unless you want it for documentation.

It might be simpler to have a bunch of const int variables and let the compiler optimize these into a single mask:

Code: [Select]

const unsigned overflow           = 1U << USIOIF;
const unsigned stop_condition = 1U << USIPF;
const unsigned data_collision   = 1U << USIDC;

Serial.print (overflow | stop_condition | data_collison, HEX);

Jack Christensen

Quote
//      (0 << USICNT3) | (0 << USICNT2) || (0 << USICNT1) | (0 << USICNT0)  // USICNT3:0: Counter Value; cleared because it is used in the loop below.


That one belongs to a category of mistakes that I never seem to be able to completely stop making!

Coding Badly

You used '||' in one place instead of '|'


Exactly!

Quote
Also, you really don't need all of the 0 << n, since that is always 0, unless you want it for documentation.


Ditto!  (Can copy-paste-modify without spending so much time with the datasheet.)

Quote
It might be simpler to have a bunch of const int variables and let the compiler optimize these into a single mask:


Thanks!

Coding Badly

That one belongs to a category of mistakes that I never seem to be able to completely stop making!


My bane are inverted logical operations (e.g. using less-than instead of greater-than).  That one was just a stuttering finger.

dc42


You used '||' in one place instead of '|'


Exactly!


I believe gcc is capable of warning about this if you add -Wlogical-op to the command line. Unfortunately, it doesn't appear that the Arduino IDE provides any easy way of adding extra options to the gcc command line.
Formal verification of safety-critical software, software development, and electronic design and prototyping. See http://www.eschertech.com. Please do not ask for unpaid help via PM, use the forum.

Coding Badly


I think version 1.5 does through one of the new configuration files.

Thanks for the tip!

Go Up
 


Please enter a valid email to subscribe

Confirm your email address

We need to confirm your email address.
To complete the subscription, please click the link in the email we just sent you.

Thank you for subscribing!

Arduino
via Egeo 16
Torino, 10131
Italy