Pages: [1]   Go Down
Author Topic: Whack-a-bug  (Read 792 times)
0 Members and 1 Guest are viewing this topic.
Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 205
Posts: 12844
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset


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:
#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 );
}
Logged

Ayer, Massachusetts, USA
Offline Offline
Edison Member
*
Karma: 54
Posts: 1846
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

You used '||' in one place instead of '|'
Code:
(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:
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);
« Last Edit: April 03, 2013, 07:58:44 am by MichaelMeissner » Logged

Grand Blanc, MI, USA
Offline Offline
Faraday Member
**
Karma: 95
Posts: 4058
CODE is a mass noun and should not be used in the plural or with an indefinite article.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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!
Logged

MCP79411/12 RTC ... "One Million Ohms" ATtiny kit ... available at http://www.tindie.com/stores/JChristensen/

Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 205
Posts: 12844
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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!
Logged

Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 205
Posts: 12844
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

United Kingdom
Offline Offline
Tesla Member
***
Karma: 224
Posts: 6613
Hofstadter's Law: It always takes longer than you expect, even when you take into account Hofstadter's Law.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

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.

Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 205
Posts: 12844
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset


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

Thanks for the tip!
Logged

Pages: [1]   Go Up
Jump to: