Whoops ... I meant "byte". If it happens that "unsigned byte" can save a cycle or two, that would really be interesting!
1 Like
Itâs good that you want to understand âwhyâ this is happening, but for the most part, youâre solving a problem that doesnât really exist if you understand and use data types âcorrectlyâ.
1 Like
I'm absolutely sure that's true. Like what is the difference between a long and a signed long?
Read up on "two's complement binary representation".
Hello aarg
I have a lot of reading up to do. And that is really the only reason I persisting to get this code running.
It's mostly wasted reading. Time spent learning to use the appropriate tools correctly, is precious. More important than pursuing oddities.
1 Like
@lastchancename @DaveX @dlloyd @anon57585045 @david_2018 @red_car
Success.
The attached code works very well. And as expected it was a logic error. But the key to handling the error was to be able to see the negative numbers and which way they are going without trying to convert 2's compliment numbers in my head to negative values.
So in the end I'm not sure if I cheated and perhaps their are better ways to accomplish this but for these long period events this seams to work.
In the case I set all the time variable to long, and recast the millis to a long variable CurrentTime. That enabled be to see what was happening on the serial monitor. With that I was able to find that my index variable was in the wrong place. And probably half of the messing around it tried with different variable types would have worked.
The attached code is a small program I created to call just the blinkspeed subroutine. It was a lot easier to work with that.
Many thanks for all your patience and time to all who participated.
const int RadPin = A0; // Radiator Input Thermister
const int CndPin = A1; // Condenser Input Thermister
const int AuxPin = A2; // Auxilary Input Not used at this time
const int LED1Pin = 7; // Set Cooling Fan LED
const int LED2Pin = 6; // Set Condenser Fan LED
const int LED3Pin = 5; // Set Aux Fan FED
const int RadSpeed = 40;
const int CndSpeed = 50;
const int AuxSpeed = 60;
const long BlinkInterval = 1000; // Blink period
void setup() {
// put your setup code here, to run once:
pinMode(RadPin, INPUT);
pinMode(LED1Pin, OUTPUT);
pinMode(LED2Pin, OUTPUT);
pinMode(LED3Pin, OUTPUT);
pinMode(LED_BUILTIN, OUTPUT);
}
void loop() {
while (!Serial); // comment this out prior to installation in vehical
BlinkSpeed();
TwidleYourThumbs();
Serial.print("*********************************************************************************************");
}
void TwidleYourThumbs() {
delay(25000);
}
void BlinkSpeed() // Blink LEDs to indicate speed called by each sensor
{
long CurrentTime; // This sets the initial time the fan PWM cycle is started
long NextBlinkOn; // this sets the next time the LEDs blink on
long NextBlinkOff; // this sets the next time the LEDs blink off
long TimeLeftOn; // Time the light has left to stay on
long TimeLeftOff; // Time the light has left to remain off
int CountRadOn = 0; // Counter for number of radiator blinks on
int CountCndOn = 0; // Counter for number of condenser blinks on
int CountAuxOn = 0; // Counter for number of Aux blinks on
int CountRadOff = 0; // Counter for number of radiator blinks off
int CountCndOff = 0; // Counter for number of condenser blinks off
int CountAuxOff = 0; // Counter for number of Aux blinks off
int RadBlinks = 0; // Initialise number of blinks based on radiator temp
int CndBlinks = 0; // Initialise number of blinks based on Condenser temp
int AuxBlinks = 0; // Initialise number of blinks based on Aux temp
int MaxBlinks = 0; // Maximum number of blinks for all channels
int index = 0;
RadBlinks = RadSpeed/10; // Number of Blinks Based on Rad Temp
CndBlinks = CndSpeed/10; // Number of Blinks Based on Cnd Temp
AuxBlinks = AuxSpeed/10; // Number of Blinks Based on Aux Temp
MaxBlinks = max(RadBlinks, CndBlinks);
MaxBlinks = max(MaxBlinks, AuxBlinks); // Find the total number of blinks required
CurrentTime = (long)millis(); // Get the initial time the blink starts
NextBlinkOn = CurrentTime;
NextBlinkOff = NextBlinkOn + BlinkInterval/2;
Serial.println();
Serial.print("Initial NextBlinkOn = "); Serial.print(NextBlinkOn); Serial.println();
Serial.print("Initial NextBlinkOff = "); Serial.print(NextBlinkOff); Serial.println(); Serial.println(); Serial.println();
do { // Do the blink loop until the max number of blinks has occured.
Serial.println();
CurrentTime = (long)millis();
Serial.print("NextBlinkOn = "); Serial.print(NextBlinkOn); Serial.println();
Serial.print("(Current Time) = "); Serial.print(CurrentTime); Serial.println();
Serial.print("(CurrentTime - NextBlinkOn) = "); Serial.print((CurrentTime - NextBlinkOn)); Serial.println(); Serial.println();
if ((CurrentTime - NextBlinkOn) >=0) {
CountRadOn = CountRadOn + 1;
CountCndOn = CountCndOn + 1;
CountAuxOn = CountAuxOn + 1;
NextBlinkOn += BlinkInterval;
index = index + 1;
Serial.print("CountRadOn = "); Serial.print(CountRadOn); Serial.println();
if (CountRadOn <= RadBlinks) {
digitalWrite(LED1Pin, HIGH); // turn the LED on
}
if (CountCndOn <= CndBlinks) {
digitalWrite(LED2Pin, HIGH); // turn the LED on
}
if (CountAuxOn <= AuxBlinks) {
digitalWrite(LED3Pin, HIGH); // turn the LED on
}
}
Serial.println();
CurrentTime = (long)millis();
Serial.print("NextBlinkOff = "); Serial.print(NextBlinkOff); Serial.println();
Serial.print("(Current Time) = "); Serial.print(CurrentTime); Serial.println();
Serial.print("(CurrentTime - NextBlinkOff) = "); Serial.print((CurrentTime - NextBlinkOff)); Serial.println(); Serial.println();
if ((CurrentTime - NextBlinkOff) >=0) {
CountRadOff = CountRadOff + 1;
CountCndOff = CountCndOff + 1;
CountAuxOff = CountAuxOff + 1;
NextBlinkOff += BlinkInterval;
Serial.print("CountRadOff = "); Serial.print(CountRadOff); Serial.println();
if (CountRadOff <= RadBlinks) {
digitalWrite(LED1Pin, LOW); // turn the LED off
}
if (CountCndOff <= CndBlinks) {
digitalWrite(LED2Pin, LOW); // turn the LED off
}
if (CountAuxOff <= AuxBlinks) {
digitalWrite(LED3Pin, LOW); // turn the LED off
}
}
} while (index < MaxBlinks);
}I hope the following sketch can help make this issue clear:
#define TAB "\t"
void setup() {
Serial.begin(115200);
Serial.print("\n--------------");
testUnsigned();
testSigned();
}
void testUnsigned() {
unsigned long x = 0xfffffff8; // allocate 32 bits to x, treat them as an unsigned long
Serial.println("\nx is an unsigned long starting at " + String(x));
for (int i = 0; i < 15; i++ ) {
x++; // Add 1 to x. If all bits as 1's, roll over to all 0's
Serial.print(i); // print row number
Serial.print(TAB);
printBin(x,32); // output 32 bits stored in x
Serial.print(TAB);
Serial.print( "unsigned long: " + String(x)); // interpret x as its default type (unsigned long)
Serial.print(TAB);
Serial.print( "signed long: ");
Serial.print((signed long) x); // interpret whatever bits are stored in x as a signed long
Serial.println();
}
}
void testSigned() {
long x = 0x7ffffff8; // allocate 32 bits to x, treat them as a signed long
Serial.println("\nx is a signed long starting at " + String(x));
for (int i = 0; i < 15; i++ ) {
x++; // Add 1 to x. If leftmost bit is 1, x is a negative number
Serial.print(i); // print row number
Serial.print(TAB);
printBin(x,32); // output 32 bits stored in x
Serial.print(TAB);
Serial.print("signed long: " + String(x));// interpret x as its default type (long, aka signed long)
Serial.print(TAB);
Serial.print("unsigned long: ");
Serial.print((unsigned long) x); // interpret whatever bits are stored in x as an unsigned long
Serial.println();
}
}
void loop () {
}
void printBin(const unsigned long num, const long nbits) {
for (long i = 0; i < nbits; i++) {
Serial.print(bitRead(num, (nbits-1) - i));
}
}
And the compiler warnings:
/Users/me/Documents/Arduino/foro4/foro4.ino: In function 'testSigned()':
/Users/me/Documents/Arduino/foro4/foro4.ino:36:6: warning: iteration 7 invokes undefined behavior [-Waggressive-loop-optimizations]
x++; // Add 1 to x. If all bits as 1's, roll over to all 0's
~^~
/Users/me/Documents/Arduino/foro4/foro4.ino:35:21: note: within this loop
for (int i = 0; i < 15; i++ ) {
~~^~~~
/Users/me/Library/Arduino15/packages/arduino/hardware/avr/1.8.4/cores/arduino/main.cpp: In function 'main':
/Users/me/Documents/Arduino/foro4/foro4.ino:36:6: warning: iteration 7 invokes undefined behavior [-Waggressive-loop-optimizations]
x++; // Add 1 to x. If all bits as 1's, roll over to all 0's
^
/Users/me/Documents/Arduino/foro4/foro4.ino:35:21: note: within this loop
for (int i = 0; i < 15; i++ ) {
^
Sketch uses 4604 bytes (1%) of program storage space. Maximum is 253952 bytes.
Global variables use 300 bytes (3%) of dynamic memory, leaving 7892 bytes for local variables. Maximum is 8192 bytes.
Smart compiler 
Not sure whats supposed to happen here. I copied it and compiled it and uploaded it but it has no output to the serial monitor.
Don't know what could be wrong. Baud rate, maybe?
This is what I see:
--------------
x is an unsigned long starting at 4294967288
0 11111111111111111111111111111001 unsigned long: 4294967289 signed long: -7
1 11111111111111111111111111111010 unsigned long: 4294967290 signed long: -6
2 11111111111111111111111111111011 unsigned long: 4294967291 signed long: -5
3 11111111111111111111111111111100 unsigned long: 4294967292 signed long: -4
4 11111111111111111111111111111101 unsigned long: 4294967293 signed long: -3
5 11111111111111111111111111111110 unsigned long: 4294967294 signed long: -2
6 11111111111111111111111111111111 unsigned long: 4294967295 signed long: -1
7 00000000000000000000000000000000 unsigned long: 0 signed long: 0
8 00000000000000000000000000000001 unsigned long: 1 signed long: 1
9 00000000000000000000000000000010 unsigned long: 2 signed long: 2
10 00000000000000000000000000000011 unsigned long: 3 signed long: 3
11 00000000000000000000000000000100 unsigned long: 4 signed long: 4
12 00000000000000000000000000000101 unsigned long: 5 signed long: 5
13 00000000000000000000000000000110 unsigned long: 6 signed long: 6
14 00000000000000000000000000000111 unsigned long: 7 signed long: 7
x is a signed long starting at 2147483640
0 01111111111111111111111111111001 signed long: 2147483641 unsigned long: 2147483641
1 01111111111111111111111111111010 signed long: 2147483642 unsigned long: 2147483642
2 01111111111111111111111111111011 signed long: 2147483643 unsigned long: 2147483643
3 01111111111111111111111111111100 signed long: 2147483644 unsigned long: 2147483644
4 01111111111111111111111111111101 signed long: 2147483645 unsigned long: 2147483645
5 01111111111111111111111111111110 signed long: 2147483646 unsigned long: 2147483646
6 01111111111111111111111111111111 signed long: 2147483647 unsigned long: 2147483647
7 10000000000000000000000000000000 signed long: -2147483648 unsigned long: 2147483648
8 10000000000000000000000000000001 signed long: -2147483647 unsigned long: 2147483649
9 10000000000000000000000000000010 signed long: -2147483646 unsigned long: 2147483650
10 10000000000000000000000000000011 signed long: -2147483645 unsigned long: 2147483651
11 10000000000000000000000000000100 signed long: -2147483644 unsigned long: 2147483652
12 10000000000000000000000000000101 signed long: -2147483643 unsigned long: 2147483653
13 10000000000000000000000000000110 signed long: -2147483642 unsigned long: 2147483654
14 10000000000000000000000000000111 signed long: -2147483641 unsigned long: 2147483655
I have been using 115200 all along. Are you sure this will work with a Nano 3.3 BLE?
The tab is a single character. What is the reason to present it as an element of an array?
No reason. My mistake. It worked here, perhaps that is messing with the Nano 33 BLE.
Better be
#define TAB '\t'
1 Like
Got it. I had to add the
while (!Serial);
line otherwise it would run befor I could hit the serial moniter.
No, that's not how it works. That large 4######### value will look something like 0xFFFFFF00 as a bit pattern, which if cast to signed value is negative since the sign bit (top bit) is a 1 (the signed value is -256 (-0x100) in this example).
The value is 32 bits, and casting unsigned <-> signed will change the effective value of the sign bit between +2147483648 <-> -2147483648
Similarly for a 16 bit integer the sign bit represents either +/-32768,
for 8 bit its +/-128
Casting doesn't change the bit pattern, just the interpretation of the bit pattern.
1 Like
This ^^
What is really interesting about it is that it changes the interpretation of the bit patten for the compiler, which then chooses the optimal opcodes for copying(!), adding, subtracting, or comparing the values accordig to that interpretation. See the assembly in the sim in Having difficulty with subtration of unsigned long - #41 by DaveX
1 Like
Which is heavier⌠a ton of bricks, or a ton of feathers ?
âŚa ton of white feathers, or a ton of black feathers ?
Update to reply #34 and #41
- Use unsigned long variables for elapsed time calculation.
- Use any unsigned type for time interval.
Several comparison methods without rollover issue:
unsigned long previousMillis = 0xFFFFFFF0;
unsigned long currentMillis = 0xFFFFFFF0;
const byte millisInterval = 3;
void setup() {
Serial.begin(115200);
while (!Serial);
for (int i = 0; i < 20; i++) {
if ((currentMillis - previousMillis) >= millisInterval) {
Serial.print(F("millis: ")); Serial.print(currentMillis); Serial.print(F(", elapsed time on left: ")); Serial.println(currentMillis - previousMillis);
}
if (millisInterval <= (currentMillis - previousMillis)) {
Serial.print(F("millis: ")); Serial.print(currentMillis); Serial.print(F(", elapsed time on right: ")); Serial.println(currentMillis - previousMillis);
}
Serial.println();
currentMillis += millisInterval;
}
}
void loop() {
}
What I find staggering, is that after all these posts, we are elaborating over one of the most fundamental understandings of programming up in ANY language.
The concepts of data types MUST be understood to develop any efficient and/or reliable code.
This what makes me despair about many of todayâs âprogrammersâ⌠theyâve never been taught or understand the very basics of computer architecture and data flow.
Just to close the loop on this I re wrote the code similar to the way I had it in the first post. That also works indicating the only real problem I was having was a logic problem, that I was able to over come once I could see the translated data in the serial monitor. I also found that the key to being able to trouble shoot this would have been a simple print statement that printed all the arguments as they were presented to the if statement.
I have two serial print lines in this code that print the arguments passed to the if statement both without and with the (signed long) and you can see the difference how they are printed out in the serial monitor.
The second line of code would have made all the difference in my ability to troubleshoot the code. And I could have avoided days of trying to figure out what 4######### means.
Serial.print("(millis() - NextBlinkOn) = "); Serial.print((millis() - NextBlinkOn)); Serial.println();
Serial.print("(signed long)(millis() - NextBlinkOn) = "); Serial.print((signed long)(millis() - NextBlinkOn)); Serial.println(); Serial.println();
The whole program is presented here:
const int RadPin = A0; // Radiator Input Thermister
const int CndPin = A1; // Condenser Input Thermister
const int AuxPin = A2; // Auxilary Input Not used at this time
const int LED1Pin = 7; // Set Cooling Fan LED
const int LED2Pin = 6; // Set Condenser Fan LED
const int LED3Pin = 5; // Set Aux Fan FED
const int RadSpeed = 40;
const int CndSpeed = 50;
const int AuxSpeed = 60;
const long BlinkInterval = 1000; // Blink period
void setup() {
// put your setup code here, to run once:
pinMode(RadPin, INPUT);
pinMode(LED1Pin, OUTPUT);
pinMode(LED2Pin, OUTPUT);
pinMode(LED3Pin, OUTPUT);
pinMode(LED_BUILTIN, OUTPUT);
}
void loop() {
while (!Serial); // comment this out prior to installation in vehical
BlinkSpeed();
TwidleYourThumbs();
Serial.print("*********************************************************************************************");
}
void TwidleYourThumbs() {
delay(25000);
}
void BlinkSpeed() // Blink LEDs to indicate speed called by each sensor
{
unsigned long NextBlinkOn; // this sets the next time the LEDs blink on
unsigned long NextBlinkOff; // this sets the next time the LEDs blink off
int CountRadOn = 0; // Counter for number of radiator blinks on
int CountCndOn = 0; // Counter for number of condenser blinks on
int CountAuxOn = 0; // Counter for number of Aux blinks on
int CountRadOff = 0; // Counter for number of radiator blinks off
int CountCndOff = 0; // Counter for number of condenser blinks off
int CountAuxOff = 0; // Counter for number of Aux blinks off
int RadBlinks = 0; // Initialise number of blinks based on radiator temp
int CndBlinks = 0; // Initialise number of blinks based on Condenser temp
int AuxBlinks = 0; // Initialise number of blinks based on Aux temp
int MaxBlinks = 0; // Maximum number of blinks for all channels
int index = 0;
RadBlinks = RadSpeed/10; // Number of Blinks Based on Rad Temp
CndBlinks = CndSpeed/10; // Number of Blinks Based on Cnd Temp
AuxBlinks = AuxSpeed/10; // Number of Blinks Based on Aux Temp
MaxBlinks = max(RadBlinks, CndBlinks);
MaxBlinks = max(MaxBlinks, AuxBlinks); // Find the total number of blinks required
NextBlinkOn = millis();
NextBlinkOff = NextBlinkOn + BlinkInterval/2;
Serial.println();
Serial.print("Initial NextBlinkOn = "); Serial.print(NextBlinkOn); Serial.println();
Serial.print("Initial NextBlinkOff = "); Serial.print(NextBlinkOff); Serial.println(); Serial.println(); Serial.println();
do { // Do the blink loop until the max number of blinks has occured.
Serial.println();
Serial.print("NextBlinkOn = "); Serial.print(NextBlinkOn); Serial.println();
Serial.print("millis() = "); Serial.print(millis()); Serial.println();
Serial.print("(millis() - NextBlinkOn) = "); Serial.print((millis() - NextBlinkOn)); Serial.println();
Serial.print("(signed long)(millis() - NextBlinkOn) = "); Serial.print((signed long)(millis() - NextBlinkOn)); Serial.println(); Serial.println();
if ((signed long)(millis() - NextBlinkOn) >=0) {
CountRadOn = CountRadOn + 1;
CountCndOn = CountCndOn + 1;
CountAuxOn = CountAuxOn + 1;
NextBlinkOn += BlinkInterval;
index = index + 1;
Serial.print("CountRadOn = "); Serial.print(CountRadOn); Serial.println();
if (CountRadOn <= RadBlinks) {
digitalWrite(LED1Pin, HIGH); // turn the LED on
}
if (CountCndOn <= CndBlinks) {
digitalWrite(LED2Pin, HIGH); // turn the LED on
}
if (CountAuxOn <= AuxBlinks) {
digitalWrite(LED3Pin, HIGH); // turn the LED on
}
}
Serial.println();
Serial.print("NextBlinkOff = "); Serial.print(NextBlinkOff); Serial.println();
Serial.print("millis() = "); Serial.print(millis()); Serial.println();
Serial.print("(millis() - NextBlinkOff) = "); Serial.print((millis() - NextBlinkOff)); Serial.println();
Serial.print("(signed long)(millis() - NextBlinkOff) = "); Serial.print((signed long)(millis() - NextBlinkOff)); Serial.println(); Serial.println();
if ((signed long)(millis() - NextBlinkOff) >=0) {
CountRadOff = CountRadOff + 1;
CountCndOff = CountCndOff + 1;
CountAuxOff = CountAuxOff + 1;
NextBlinkOff += BlinkInterval;
Serial.print("CountRadOff = "); Serial.print(CountRadOff); Serial.println();
if (CountRadOff <= RadBlinks) {
digitalWrite(LED1Pin, LOW); // turn the LED off
}
if (CountCndOff <= CndBlinks) {
digitalWrite(LED2Pin, LOW); // turn the LED off
}
if (CountAuxOff <= AuxBlinks) {
digitalWrite(LED3Pin, LOW); // turn the LED off
}
}
} while (index < MaxBlinks);
}
A sample of the serial output is presented here:
21:58:00.523 ->
21:58:00.523 -> Initial NextBlinkOn = 14935
21:58:00.523 -> Initial NextBlinkOff = 15435
21:58:00.523 ->
21:58:00.523 ->
21:58:00.523 ->
21:58:00.523 -> NextBlinkOn = 14935
21:58:00.523 -> millis() = 14947
21:58:00.523 -> (millis() - NextBlinkOn) = 15
21:58:00.523 -> (signed long)(millis() - NextBlinkOn) = 18
21:58:00.523 ->
21:58:00.523 -> CountRadOn = 1
21:58:00.523 ->
21:58:00.523 -> NextBlinkOff = 15435
21:58:00.523 -> millis() = 14964
21:58:00.523 -> (millis() - NextBlinkOff) = 4294966828
21:58:00.523 -> (signed long)(millis() - NextBlinkOff) = -465
21:58:00.523 ->
21:58:00.523 ->
21:58:00.523 -> NextBlinkOn = 15935
21:58:00.523 -> millis() = 14979
21:58:00.523 -> (millis() - NextBlinkOn) = 4294966343
21:58:00.523 -> (signed long)(millis() - NextBlinkOn) = -950
21:58:00.523 ->
21:58:00.523 ->
21:58:00.523 -> NextBlinkOff = 15435
21:58:00.523 -> millis() = 14994
21:58:00.523 -> (millis() - NextBlinkOff) = 4294966858
21:58:00.523 -> (signed long)(millis() - NextBlinkOff) = -435
Notice - 435 is a lot easier to read and figure out whats going on than 4294966858.
I believe the only real difference between this code and the one published in post 48 is that this one will run for 50 days instead of 25.