A naughty IF [SOLVED]

Projects Programming
1

Hi, I show you the code and then I make my question…

#include <LiquidCrystal.h>

LiquidCrystal lcd(8,9,10,11,12,13);
int rpm = 0;
int rpmnw = 0;
int potpin = 0;

void setup () {
  lcd.begin(16,2);
}

void loop () {
  rpm = analogRead(potpin);
  rpm = map(rpm,0,1023,65,200);
  rpm *= 10;
  if (rpm != rpmnw) {
    rpmnw = rpm;
    char buf[8];
    sprintf(buf,"%4d RPM ",rpm);
    lcd.write(buf);
  }
  delay(2000);
  lcd.clear();
}

So I my code read the 10K potentiometer and map it to a desirable value then format it with sprintf and print it to lcd, but I want it to happens only when I make changes on the pot so I add an statement to the code to try to make it… but it doesn't work… it keeps displaying the value at the lcd and I don't know why…

Then my question is where is the mistake?

2

It looks right to me.

A long shot, but try making your buffer bigger

    char buf[8];
    sprintf(buf,"%4d RPM ",rpm);

a buffer of size 8 only allows 7 characters + nul. " RPM " is already 5 characters. It is possible that the sprintf is clobbering memory after your character string, which could be part of your rpmnw variable.

You could try printing the variable at various points in the code to see what the value is and where it is changiing.

Also, potpin should probably be declared constant and could be a byte or uint8_t type.

3

analogRead is unlikely to return exactly the same value each time, due to noise on the chip. Maybe check for +/- 10 units rather than exact equality.

4 
int potpin = 0;

Pin 0 is a digital pin.

5

Hi,
Try

int potpin = A0;

Tom.... :slight_smile:

6 
  rpm = map(rpm,0,1023,65,200);
  rpm *= 10;

As opposed to mapping to 650 to 2000?

Pin 0 is a digital pin.

There's also an analog pin 0. Context matters.

TomGeorge:
Hi,
Try

int potpin = A0;

Tom.... :slight_smile:

A0 is the name to use when using the analog pin as a digital pin. There is NOTHING wrong with OPs code, as far as reading from the analog pin.

7

aarg:

int potpin = 0;

Pin 0 is a digital pin.

And it's also an analogue pin, when used in conjunction with analogRead, like it is here.

8

PaulS:

  rpm = map(rpm,0,1023,65,200);

rpm *= 10;



As opposed to mapping to 650 to 2000?

You might use the first way if you want the units digit to always be 0.

9

Mr Gammon just solved your problem.[quote author=Nick Gammon link=msg=2335457 date=1438167316]
analogRead is unlikely to return exactly the same value each time, due to noise on the chip. Maybe check for +/- 10 units rather than exact equality.
[/quote]

10

Fransie:
Mr Gammon just solved your problem.

but if it was the case the value printed should change but it keeps constant in the lcd…

11

marco_c:
It looks right to me.

A long shot, but try making your buffer bigger

    char buf[8];

sprintf(buf,"%4d RPM ",rpm);




a buffer of size 8 only allows 7 characters + nul. " RPM " is already 5 characters. It is possible that the sprintf is clobbering memory after your character string, which could be part of your rpmnw variable.

You could try printing the variable at various points in the code to see what the value is and where it is changiing.

Also, potpin should probably be declared constant and could be a byte or uint8_t type.

I will try this…

Edit: and yes it was the problem, I change the code:

    char buf[8];
    sprintf(buf,"%4d RPM",rpm);

and now it works, how a F*** wrong blank can ruin a entire code…

12

but if it was the case the value printed should change but it keeps constant in the lcd…

You COULD Serial.print() the old and new values of rpm, to see if they are, indeed, the same.

13

You are throwing away detail there by doing the map. I tested every possible value:

void setup ()
  {
  Serial.begin (115200);
  Serial.println ();
  
  int rpm;
  int foo;
 
  for (rpm = 0; rpm < 1024; rpm++)
    {  
    foo = map(rpm,0,1023,65,200);
    foo *= 10;
    Serial.print ("rpm = ");
    Serial.print (rpm);
    Serial.print (", foo = ");
    Serial.println (foo);
    }
    
  }  // end of setup

void loop ()
  {
  }  // end of loop

Results:

rpm = 0, foo = 650
rpm = 1, foo = 650
rpm = 2, foo = 650
rpm = 3, foo = 650
rpm = 4, foo = 650
rpm = 5, foo = 650
rpm = 6, foo = 650
rpm = 7, foo = 650
rpm = 8, foo = 660
rpm = 9, foo = 660
rpm = 10, foo = 660
rpm = 11, foo = 660
rpm = 12, foo = 660
rpm = 13, foo = 660
rpm = 14, foo = 660
rpm = 15, foo = 660
rpm = 16, foo = 670
rpm = 17, foo = 670
rpm = 18, foo = 670
rpm = 19, foo = 670
rpm = 20, foo = 670
rpm = 21, foo = 670
rpm = 22, foo = 670
rpm = 23, foo = 680
...
rpm = 999, foo = 1960
rpm = 1000, foo = 1960
rpm = 1001, foo = 1970
rpm = 1002, foo = 1970
rpm = 1003, foo = 1970
rpm = 1004, foo = 1970
rpm = 1005, foo = 1970
rpm = 1006, foo = 1970
rpm = 1007, foo = 1970
rpm = 1008, foo = 1980
rpm = 1009, foo = 1980
rpm = 1010, foo = 1980
rpm = 1011, foo = 1980
rpm = 1012, foo = 1980
rpm = 1013, foo = 1980
rpm = 1014, foo = 1980
rpm = 1015, foo = 1980
rpm = 1016, foo = 1990
rpm = 1017, foo = 1990
rpm = 1018, foo = 1990
rpm = 1019, foo = 1990
rpm = 1020, foo = 1990
rpm = 1021, foo = 1990
rpm = 1022, foo = 1990
rpm = 1023, foo = 2000
14

Can we reword this thread title from "A naughty IF" to "A naughty BUFFER OVERFLOW"?