if (-1 > 4) evaluates to true?

Here's the part that seems to be making trouble:

if (i > sizeof(ledPin)-1){ // why doesn't sizeof(ledPin)-1 work?
    Serial.println(sizeof(ledPin)-1);
    Serial.println(i);
    i = 0;
  } else if (i < 0) { 
      i = sizeof(ledPin)-1;
  }

Below is the entire code.
(When running with direct = -1:)
The println's show that sizeof(ledPin)-1 is 4 and i is -1 (as expexted) but then how the heck does that block even execute?
The condition states that i must be bigger than the biggest index number in ledPin[] for this block to execute?
The result of this problem is that the "else if" block never executes.

Also: for some reason the led on ledPin[0] doesn't blink when this goes wrong, which I do not understand.
The code works fine when "sizeof(ledPin)-1" is replaced with "4".

const byte ledPin[] = {4, 5, 6, 7, 12};
const byte potenPin = 2;
const byte buttn = 2;
int direct = 1;

void blink(byte n); 

void setup() {                
  for (int i =0; i< sizeof(ledPin); i++) {
  pinMode(ledPin[i], OUTPUT);
  pinMode (buttn, INPUT);
  Serial.begin(9600);
  }
}

int i =0;
void loop() {
  blink(i);
  
  if(digitalRead(buttn) == HIGH) {
    while(digitalRead(buttn) == HIGH) {
    delay(25); //wait for button release
    }
    direct = (direct > 0) ? -1:1;
  }
  
  i += direct;
if (i > sizeof(ledPin)-1){ // why doesn't sizeof(ledPin)-1 work?
    Serial.println(sizeof(ledPin)-1);
    Serial.println(i);
    i = 0;
  } else if (i < 0) { 
      i = sizeof(ledPin)-1;
  }
}

void blink(byte n) {
  short
  ledDelay= analogRead(potenPin)/4 +50;
  digitalWrite(ledPin[n], 1);
  delay(ledDelay);
  digitalWrite(ledPin[n], 0) ;
}

The only thing that I can think of is that sizeof() returns an unsigned value. How can the size of something be negative? It can't, so unsigned is the proper return type. Mixed mode comparisons lead to all kinds of issues.

Put the result of the call to sizeof() into an int. It should be const, anyway, since the size of the array can not change.

I think what is happening is that you are mixing signed and unsigned integers.
The size of a string can't be negative so sizeof actually returns an unsigned integer result - on top of that it may return an unsigned LONG but the documentation doesn't say. Subtracting one from it leaves the result as unsigned.
Then you try to compare a signed integer (i) with the unsigned result. If i is -1, as an unsigned integer that is 0xFFFF => +65535 which is the largest unsigned integer that can be represented in 16 bits and that is going to be larger than any sizeof minus one.

Pete

Ahh, so when comparing variables it is not taken in to account whether they are signed?
That would explain it.

There is still something I don't get though.

If I put sizeof(ledPin)-1 into a constant it works for byte (which I'm guessing is always unsigned) and signed int but not for unsigned int?

It seems logical that comparisons between signed ints works, and it makes sense that it doesn't between signed and unsigned ints.
But why does it work between a byte and a signed int? :slight_smile:

When a binary operator (such as '<') is operating on both signed int and unsigned int the signed value (-1) is 'promoted' to unsigned int (65535).

You can cast the result of the sizeof:

if (i > ((int)sizeof ledPin) - 1){

Probably shouldn't be doing Serial.begin(9600) five times.

void setup() {                
  for (int i =0; i< sizeof(ledPin); i++) {
  pinMode(ledPin[i], OUTPUT);
  pinMode (buttn, INPUT);
  Serial.begin(9600);
  }
}

The "byte" you refer to is probably a signed "char". There is also the data type "unsigned char" which would again give you problems.

Ahh, so when comparing variables it is not taken in to account whether they are signed?

No, that's not the right way to look at it. If both sides of the comparison are signed integers then the compiler uses a signed comparison. If both sides are unsigned then it uses unsigned comparison. But you have told the compiler to compare a signed int and an unsigned int. It has to make a decision as to how to do that. If it treats the unsigned int as being signed, any number above 32767 (for 16-bit int) will be interpreted as a negative number and you will also get weird results. The compiler was written such that when faced with this decision, it will generate code to treat them both as unsigned. Some compilers (e.g. the MSVC I use on Win 7) will give a warning when it encounters this sort of situation to let you know that your code could fail.

Pete

Hmm, okay. But I still don't get why "byte > signed int" works then?

Does anyone know a good resource about this? Wikipedia isn't exactly newbe-friendly with this kind of subject.
Not that I'm not grateful for your answers, I just don't want to waste your time : )

As I said, if by "byte" you mean "char" (or do you mean the old byte keyword in 0023 and earlier?) then it is signed, so a byte containing -1 will be less than an int containing +4.
I don't know offhand of a source which explains this well.

Pete

I'm talking about the byte type (which should pretty much be an unsigned char).
I don't know whether its deprecated, but it's in the arduino code reference: Byte.
I'd expect it to give the same problem as an unsigned int, when compared to a signed int, but it doesn't?

In that case "byte" can represent numbers from 0 to 255. "int" can represent numbers in the range -32768 to +32767.
And here you run into another problem. Not only is there an unsigned/signed mismatch but the size of the datatypes is different. A byte is only 8 bits whereas the int is 16-bits. Again the compiler has to make a decision about what to do in this case and since a signed int can comfortably represent the entire range of 0-255, it presumably "promotes" the unsigned byte to signed and away it goes.

Pete

Ahh, I thought your earlier post meant to say the compiler always treats both values as unsigned, when faced with a signed and unsigned value.
I guess that's not what you were saying : )
So it chooses based on the types involved?
"unsigned char > signed int" becomes "signed char > signed int" - because the signed int can contain 0-255 while;
"unsigned int > signed int" becomes "unsigned int > unsigned int" - which is troublesome, because an unsigned int would interpret 65536 as 65536, when seeing as it originates from a signed int, 65536 atually means -1. But if it had converted it to "signed int > signed int" the problem would just occur the other way arround.

Or am I completely wrong here?

So it chooses based on the types involved?

Yes, although I may be misleading you a bit when I've said that the compiler "chooses". Actually, the definition of the language defines what the compiler must do when faced with comparisons of signed/unsigned etc.

an unsigned int would interpret 65536 as 65536

No. First an unsigned int can only go up to 65535 but an unsigned 16-bit int whose hex value is 0xFFFF is 65535 - it doesn't interpret anything.
But the same hex value of 0xFFFF when used in signed arithmetic, is -1.

But if it had converted it to "signed int > signed int" the problem would just occur the other way around

Yes, it's a win some - lose some situation. When you have programmed for a long time this sort of thing becomes second nature, although as I mentioned, it helps to have a compiler warn you that what you're doing might not work out.
In fact, I feel it is very good practice that wherever a compiler gives a warning, you change your code so that it is correct and the compiler doesn't issue any warnings at all.

Pete

In fact, I feel it is very good practice that wherever a compiler gives a warning, you change your code so that it is correct and the compiler doesn't issue any warnings at all.

The gcc compiler that the Arduino IDE uses DOES give warnings. The Arduino team has decided, for undisclosed reasons, not to show them to us.

I wondered about that. I was pretty sure that gcc did that but I rarely use it outside the Arduino environment.

Thanks Paul.

Pete

PaulS:

In fact, I feel it is very good practice that wherever a compiler gives a warning, you change your code so that it is correct and the compiler doesn't issue any warnings at all.

The gcc compiler that the Arduino IDE uses DOES give warnings. The Arduino team has decided, for undisclosed reasons, not to show them to us.

Keeping SHIFT key pressed while clicking on the compile button should output verbose messages. There's also a simple way to force the compiler to report each warning as error, but I don't remember it right now. Please search the old forum for something like compiler warning error Wall, there was a very interesting thread about this subject awhile ago, which explained also why warnings were "hidden away" from the user.

Bladtman24 asked about a reference resource...

The very best is the ISO/IEC C-language standard, which can be downloaded in draft form as a PDF from http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf

Thanks : )
Could you maybe point me towards a header or keyword? I looked under comparisons, conversion and searched the document for "unsigned arithmetic" etc.
I guess I'm not too familiar with the terminology.

Holding the shift button doesn't appear to change anything when compiling. I think I'll look into compiling/uploading from the CLI, not a big fan of the arduino IDE anyway : )

I did a quick search on 'expressions convert' and it appears that most of the "meat" is in section 6.3 (Conversions), but ISTR that there's additional/related info attached elsewhere.

I'm a new Arduino user, and have some well-established development preferences [ translation: I can change if I have to, but would prefer not to. :slight_smile: ] I'll be grateful if you share what you learn.

Thank you once more :slight_smile:
What I learn about compiling and uploading from the command line?
If I can figure out how, certainly : )

PaulS:
The gcc compiler that the Arduino IDE uses DOES give warnings. The Arduino team has decided, for undisclosed reasons, not to show them to us.

They would just confuse us, Paul. After all it's bad enough dealing with errors, without having those pesky warnings as well. :wink: