Hello all,
This is my first full sketch for a project and from the testing i have done it seems to work well, however im looking for advice on ways to get better and was wondering if there were different approaches that would work better in the programming also just curious about general advice and knowledge on technique. This project is using one 22mm industrial 3 way switch to control panel/pendant control and then has two other separate sets of 22mm momentary push-buttons one panel mount and one for remote control. The panel/remote buttons control relays which send signal to a vfd and an ac solenoid.
/*Board used Arduino Mega R3
Reads position of 3 way selector switch to control remote or panel function
Monitors input of push-buttons based upon seleector switch location
Outputs signal to relay pins when buttons are activated
*/
//Define input pins
#define pendant 50
#define panel 52
#define uppendant 53
#define downpendant 51
#define uprelay A1
#define downrelay A2
#define uppanel 49
#define downpanel 48
void setup()
{
Serial.begin(9600);
//Assign inputs to defined pins
pinMode(pendant, INPUT_PULLUP);
pinMode(panel, INPUT_PULLUP);
pinMode(uppendant, INPUT_PULLUP);
pinMode(downpendant, INPUT_PULLUP);
pinMode(uppanel, INPUT_PULLUP);
pinMode(downpanel, INPUT_PULLUP);
//Assign outputs to defined pins
pinMode(uprelay, OUTPUT);
pinMode(downrelay, OUTPUT);
//Write relay pins high in setup to avoid them going low in loop
digitalWrite(uprelay, HIGH);
digitalWrite(downrelay, HIGH);
}
void loop()
{
//Define functions
int pendantselect = digitalRead(pendant);
int panelselect = digitalRead(panel);
int pendantup = digitalRead(uppendant);
int pendantdown = digitalRead(downpendant);
int panelup = digitalRead(uppanel);
int paneldown = digitalRead(downpanel);
//used to read what position 3 way switch is in
if (pendantselect == LOW && panelselect == HIGH)
{
Serial.println("pendant");
if (pendantup == LOW)
{
Serial.println("up");
digitalWrite(uprelay, LOW);
}
else
{
digitalWrite(uprelay, HIGH);
}
if (pendantdown == LOW)
{
Serial.println("down");
digitalWrite(downrelay, LOW);
}
else
{
digitalWrite(downrelay, HIGH);
}
}
//used to read what position 3 way switch is in
if (pendantselect == HIGH && panelselect == LOW)
{
Serial.println("panel");
if (panelup == LOW)
{
Serial.println("up");
digitalWrite(uprelay, LOW);
}
else
{
digitalWrite(uprelay, HIGH);
}
if (paneldown == LOW)
{
Serial.println("down");
digitalWrite(downrelay, LOW);
}
else
{
digitalWrite(downrelay, HIGH);
}
}
}
void loop()
{
//Define functions
int pendantselect = digitalRead(pendant);
They declare variables and assign values to them.
If you use INPUT_PULLUP, logic is reversed. Instead of using LOW, your code will be a little more readable when you add the below near the top of your code
#define ISPRESSED LOW
and where you test your buttons for LOW, use ISPRESSED
// if pendantselect is pressed and pannelselect is not pressed
if (pendantselect == ISPRESSED && panelselect != ISPRESSED)
You can use a different name for ISPRESSED; ISPRESSED refers to buttons, usually not to switches.
int pendantselect = digitalRead(pendant);
int panelselect = digitalRead(panel);
int pendantup = digitalRead(uppendant);
int pendantdown = digitalRead(downpendant);
int panelup = digitalRead(uppanel);
int paneldown = digitalRead(downpanel);
these are not integers, they are boolean or 'bool'
int panelselect = digitalRead(panel);
int pendantup = digitalRead(uppendant);
int pendantdown = digitalRead(downpendant);
int panelup = digitalRead(uppanel);
int paneldown = digitalRead(downpanel);
these are not integers, they are boolean or 'bool'
{
//Define functions
int pendantselect = digitalRead(pendant);
They declare variables and assign values to them.
If you use INPUT_PULLUP, logic is reversed. Instead of using LOW, your code will be a little more readable when you add the below near the top of your code
#define ISPRESSED LOW
and where you test your buttons for LOW, use ISPRESSED
// if pendantselect is pressed and pannelselect is not pressed
if (pendantselect == ISPRESSED && panelselect != ISPRESSED)
You can use a different name for ISPRESSED; ISPRESSED refers to buttons, usually not to switches.
thankyou for the advice i am going to make the changes now i appreciate your input.
So im confused are they correct being defined as int or should the be boolean or something else? Im still a little unsure of how and what to define things as.
When giving names to constants there are advantages to using 'const' variables instead of preprocessor #defines. It allows the compiler to do better type checking and to detect writing into a constant.
I like to use an initial uppercase on global variables/constants to make the code easier to read.
I like to append the word "Pin" to the names of constants representing Arduino pin numbers.
sterretje:
If you use INPUT_PULLUP, logic is reversed. Instead of using LOW, your code will be a little more readable when you add the below near the top of your code.
I am a little unsure of what you mean by using INPUT_PULLUP because in the code i did im just wondering where you are suggesting i use it at. that was one problem i had was i didnt like writing everything low but i coulndt figure out how to do it and be able to have the arduino know weather or not it was for sure high or for sure low i fell like it actually needed pulldown i think but i dont want extra hardware
const byte buttonPin = 48; // wire a button that connects 48 to GND when pressed
pinMode (buttonPin, INPUT_PULLUP);
if (digitalRead(buttonPin) == LOW){
// buttonPin has been pressed, do something
}
else {
// buttonPin is not pressed, perhaps do something else
}
johnwasser:
When giving names to constants there are advantages to using 'const' variables instead of preprocessor #defines. It allows the compiler to do better type checking and to detect writing into a constant.
I like to use an initial uppercase on global variables/constants to make the code easier to read.
I like to append the word "Pin" to the names of constants representing Arduino pin numbers.
thankyou much but if i may ask using a #define doesnt use memory where as using const byte does correct? what are the advantages and dissadvantages of using define vs const byte? Im not questioning your knowledge im just trying to learn the correct things and get into the habbit of doing things correct
const byte buttonPin = 48; // wire a button that connects 48 to GND when pressed
pinMode (buttonPin, INPUT_PULLUP);
if (digitalRead(buttonPin) == LOW){
// buttonPin has been pressed, do something
}
else {
// buttonPin is not pressed, perhaps do something else
}
Bjohn118:
thankyou much but if i may ask using a #define doesn't use memory where as using const byte does correct?
In most cases they will use roughly the same amount of memory. The compiler knows enough not to set aside room for a constant if it doesn't need to. On the other hand, a string constant will often use MORE memory as a #define since each use will be saved as a separate string in memory:
const char message[] = "This is a long string used several times.";
// This will store the data once and pass the same address three times:
LCD.write(message);
LCD.write(message);
LCD.write(message);
#define message "This is a long string used several times.";
// This will store the data three times and pass the three addresses once each:
LCD.write(message);
LCD.write(message);
LCD.write(message);
thank you for everyone's assistance just from this small topic i have learned a lot and people have been very helpful i am currently adding an i2c display to the project and am using provided examples and also changing previous code and comments to be better understood once again thank you for the help and guidance
