Hello and tips please

Hi all,

I’m new to all this electronic stuff and over the last week have bought a starter kit from RS.
I’m an engineer working with CNC controls and like to programme most of the machines in Macro programming so I have a little experience with variables and different statements, so although the code is different, there is a similarity in the thought process.

I’ve been working through a few tutorials to get familiar with the UNO.

I completed the P09_motorized_pinwheel example and continued to try to modify it.

I’ve moved on from just a button to start and stop the motor. I now have a potentiometer to set a run time, a start button to begin the cycle and a stop button to break the cycle. All with an LCD screen to display the status.

All this has been a big learning curve and has taken me several hours in development, research and debugging. I kept finding scenarios where I would get an undesirable result. For example, I found if the motor was running & I pressed the start button again, it would reset the timer. A simple IF this AND IF this sorted that one.

I struggled quite a bit with the millis calculations. My head just couldn’t quite grasp with the way it was explained. I finally broke it down into, "get a start time ONCE where it wouldn’t be read again (in the start button pressed IF), get a NOW time just before the calculation that was always read while the code was running. Then the calculation was simple, time now - start time = elapsed time, if that >= timer then we’ve done.

I’m really pleased with the outcome and everything appears to work as expected. If anybody would look through my code and give me any pointers on the layout or if I’ve over complicated things it would be appreciated. If I’m doing it wrong I would like to know sooner rather than later before getting run away with bad habits.

so hopefully I’ve attached the code correctly before hitting post!!!
apologies if not.

Thanks in advance
JP

// named constants for the switch and motor pins
const int switchOnPin = 2; // the number of the ON switch pin
const int switchOffPin = 7; // the number of the OFF switch pin
const int motorPin =  9; // the number of the motor pin
int const potPin = A0; // the number of the POT analouge pin
int potVal; // variable for POT position
int timer; // variable for timer, MAPPED to potVal
int countDown; // variable for countdown to eliminate change to pot value when running
int time;
#include <LiquidCrystal_I2C.h> // set up LCD
LiquidCrystal_I2C lcd(0x27, 16, 2); // LCD 16x2
int switchOnState = 0;  // variable for reading the ON switch's status
int switchOffState = 0;  // variable for reading the OFF switch's status
int motorState = 0;  // variable for motor state
unsigned long millisNow = 0; // the NOW ms for checking
unsigned long millisStart = 0; // the ms time stamp at start of ON press
unsigned long millisTimer = 0; // the ms tme to run for


void setup() {
 
  Serial.begin(9600); // set up serial comms
  
  pinMode(motorPin, OUTPUT); // initialize the motor pin as an output:
 
  pinMode(switchOnPin, INPUT); // initialize the both switch pins as an input:
  pinMode(switchOffPin, INPUT);
 
  lcd.begin(); // start LCD, clear, set curser to top left, print first line
  lcd.clear();
  lcd.setCursor(0,0);
  lcd.print("Timer = Seconds");
  lcd.setCursor(0,1);
  lcd.print("Motor OFF       ");
  delay(3000); // wait for 3 seconds to show the timer will be in Seconds
  lcd.setCursor(0,0);
  lcd.print("Timer =        "); // used to clear the last portion of the first line without using lcd.clear()
}

void loop() {
  potVal=analogRead(potPin); // read pot position
  timer=map(potVal,0,1023,0,30); // map pot position between 0-30 Change for varing time spans
  lcd.setCursor(8,0); // display the time at which the pot is set
  lcd.print(timer);
  if(timer<10){ // this is used to clear the caracters behind a lower time (if over print 123 with 23, youd get 233 blank out the last one
    lcd.setCursor(9,0);
    lcd.print("   ");
  }
  if(timer<100){
    lcd.setCursor(10,0);
    lcd.print("  ");
  }
  if(timer<1000){
    lcd.setCursor(11,0);
    lcd.print(" ");
  }
 
  switchOnState = digitalRead(switchOnPin); // read the state of the switch value:

// IF the motor is stopped AND the start button is pressed. To stop the timer being reset when running
  if(motorState == 0){
    if (switchOnState == HIGH) {  // check if the ON switch is pressed.
      millisStart = millis(); // get time stamp millis
      countDown = timer;
      digitalWrite(motorPin, HIGH);  // turn motor on:
      motorState = 1;
      lcd.setCursor(0,1); // set cursor at start to stop scrolling
      lcd.print("Motor ON "); // LCD says motor ON   
    }
  }
    unsigned long millisNow = millis();  // get Millis now
    
// If the motor is running
   if(motorState == 1){ // check to see if motor is ON 
    millisTimer = countDown *1000; // set countdown from timer value(seconds) to ms
    lcd.setCursor(10,1);
    lcd.print("T-");
    lcd.print(((millisTimer - (millisNow - millisStart))/1000)+1);
    lcd.print("   "); 
   }

// If the counrdown has been reached
  if (millisNow - millisStart >= millisTimer){ // Now - Start = elapsed time, has it reached Timer?
    digitalWrite(motorPin, LOW); // if it has then turn off motor
    motorState = 0;
    lcd.setCursor(0,1); // set cursor at start to stop scrolling
    lcd.print("Motor OFF       ");
  }

  switchOffState = digitalRead(switchOffPin); // Read the OFF switchs

// If the OFF switch is pressed  
    if(switchOffState == HIGH){
    digitalWrite(motorPin, LOW);
    motorState = 0;
    lcd.setCursor(0,1); // set cursor at start to stop scrolling
    lcd.print("Motor OFF       ");
}
  Serial.print(millisStart);
  Serial.print(" : ");
  Serial.print(millisNow);
  Serial.print(" : ");
  Serial.println(millisTimer);
  
  }

@john-paul, Welcome.

As yours is a programming question I am suggesting to the Moderator to move it to the Programming section.

It seems as if you have figured things out correctly, but I am not good at reviewing code.

By the way the word "pointer" has a specific technical meaning in C++ programming and the words "tips" or "suggestions" would be more suitable for your Title. You can edit the Title if you modify your Original Post - but in this case it's not the end of the world if you don't.

...R
Several Things at a Time
Using millis() for timing. A beginners guide

Thanks.

Title updated.

john-paul:
If anybody would look through my code and give me any pointers on the layout or if I've over complicated things it would be appreciated.

your code looks good.

but like a lot of code using LCDs, there are bunches of "lcd." calls throughout the code that could be simplified making the code easier to read by do two things.

create a sub-function with string arguments (char *) to print strings on the LCD. of course this won't work if the code needs to update front/back half of a line.

use sprintf() to format the strings instead of printing them piecemeal

why this seems unnecessary for a 100 line code, it becomes more helpful as programs become bigger

Both good suggestions.

I've just had a quick read up on them and the sprintf() looks very useful on my 16x2 display.

I'm interested in the char() and the relationship with ASCII. I'll read more on that also.

Thank you for your reply.
jp

It seems to me that this section of code runs continuously, whenever the timer is not "running":

 // If the counrdown has been reached
  if (millisNow - millisStart >= millisTimer) { // Now - Start = elapsed time, has it reached Timer?
    digitalWrite(motorPin, LOW); // if it has then turn off motor
    motorState = 0;
    lcd.setCursor(0, 1); // set cursor at start to stop scrolling
    lcd.print("Motor OFF       ");
  }

So you'll be writing, Motor OFF Motor OFF Motor OFF Motor OFF... to the display most of the time. It just seems weird. Did you perhaps intend:

 // If the motor is running and the countdown has been reached
  if (motorState == 1 and millisNow - millisStart >= millisTimer) { // Now - Start = elapsed time, has it reached Timer?

??

aarg:
Did you perhaps intend:

 // If the counrdown has been reached

if (millisNow - millisStart >= millisTimer and motorState == 1) { // Now - Start = elapsed time, has it reached Timer?


??

YES!!!
That is exactly what I need... "and" I didn't realise that. I also mentioned it in the first post where I put it in two "if" statements that could also use the "and"

so

  if(motorState == 0){
    if (switchOnState == HIGH) {

now becomes...

 if (switchOnState == HIGH and motorState == 0) {

Thank you, that will help a lot.
jp

FYI, 'or' and 'not' are also available. :slight_smile:

Brilliant!
so much to learn. I've only managed to get a motor to spin and my head is hurting...