Did I miss something ?

Projects Programming
1

Hello,

First of all, I'm a beginner with Arduino and this is my first post, so hello everybody :slight_smile:

I've a question about the following code.

It work pretty well until millis() reaches 65535, then a call to aBtn.read() always return 1 since millis() reaches 131071.
It smell like a uint16 overflow but I can't definitely find where it comes from :confused:

Does anybody has a clue ?

Regards,
Damien

// SimpleButton.h
#ifndef SimpleButton_h
#define SimpleButton_h

#include <stdlib.h>
#include <stdio.h>
#include <stdint.h>
#include <WProgram.h>

class SimpleButton {
 private:
  unsigned long lastReadTime;
  uint8_t lastReadValue, pin, temp;
 public:
  SimpleButton(uint8_t);
  uint8_t read();
};
#endif

// SimpleButton.cpp
#include "SimpleButton.h"

#include <stdlib.h>
#include <stdio.h>
#include <stdint.h>
#include <WProgram.h>
#include <wiring.h>

SimpleButton::SimpleButton(uint8_t btnPin) {
  pin = btnPin;
}

uint8_t SimpleButton::read() {
  if (millis() > (lastReadTime + 20)) {
    temp = digitalRead(pin);
    if (lastReadValue == 0 && temp == 1) {
      lastReadValue = 1;
      lastReadTime = millis();
      return 1;
    }
    if (temp == 0) {
      lastReadValue = 0;
      lastReadTime = millis();
    }
    return 0;
  }
}
2

I can't see where the uint16 overflow happens, but shouldn't you have a return statement also in the top scope of the SimpleButton::read() function? Now it returns a defined value only when (millis() > (lastReadTime + 20)) is true.

3

Damn, you're right !

That should be the issue. I'll test it tonight.
I suppose, correct me if I'm wrong, that the return value is read from a register and as the function doesn't set it, it's kinda random (or depending of the last function call return value) ?

I'm corrupted by the "Not all code returns a value" error in some IDEs :smiley:

Regards,
Damien

4

Yes it returns a random value, at least on the other compilers I have worked with. (What ever happens to be in the register that holds the return value).

5

Btw, if you what to be sure that the statement "(millis() > (lastReadTime + 20)) " works when millis() actually overflows in about 50 days, you may need to change it to: "(millis() - lastReadTime > 20) ".

6

This was the next step, but i think I'll do it more like
if (millis() < lastReadTime && millis() > 20) {
lastReadTime = 0;
}

before the "millis() > (lastReadTime + 20)" test
I suspect yours to not handle a lastReadTime on the late (when LRT is greater than millis()), no ?

Regards,
Damien

7

The best way to account for rollover is what was already given:
(millis() - lastReadTime > 20)

There are threads to explain how the sign math works. The short story is: this type of subtraction will always work.

8

I apologize, I forget the fact those longs where unsigned.

Thanks,
D.