Notifications
Clear all

#newb Could I simplify my code?

5 Posts
4 Users
0 Reactions
2,452 Views
Pete Bevan
(@pete-bevan)
Member
Joined: 4 years ago
Posts: 1
Topic starter  

   
Quote
(@zeferby)
Member
Joined: 5 years ago
Posts: 355
 
Posted by: @pete-bevan

The switches are momentary action SPST push button types and use the internal pull up resistor on the Arduino

Hello Pete, before i check your code, a first question : why not wire the "symetrical" push buttons in "parallel pairs" if you don't need to know from which side a button has been pushed : 1a and 1b coud share a pin, etc...

Eric


   
ReplyQuote
byron
(@byron)
No Title
Joined: 5 years ago
Posts: 1131
 
Posted by: @pete-bevan

1. Is there a better way to debounce without adding a lot of extra code?

2. Can I simplify this code, there seems to be a lot of repeated statements and similar code throughout the program?

Sometimes for a short program with limited goals its actually can be better in terms of readability to simply repeat similar code, but I have written a small program to mimic the setting and toggling of relays and LED's pins on button pressing for amusement and comparison.

First thing I should point out is that when using a lot of pins for lots of LED's then you should be looking at multiplexing.  Here is a link to an article on this 

https://www.instructables.com/id/Multiplexing-with-Arduino-and-the-74HC595/

You will find others with some googling.

I don't go into multiplexing and my program uses lots of pins and assumes the following.

As I understand it, you have five relays that can be be switched on or off.  The on/off state of each relay is indicated by 2 LED's on 2 panels. (so 4 LED's for each relay).  Each pair of LED's on a  panel represents one for the on state and one for the off state and the appropriate LED will be turned on or off to indicate the state of the relay.   Each relay has 2 momentary push button switches, one on each panel and the press of any one of the pair will toggle the relay and toggle the LED's on both panels.

As I though it would be handy to group together the pin assignments for all LED's related to each relay I created a structure that can hold the pin assignments and their current status (hight or low).

A button push relating to a relay then tiggers the relay and associated LED's to go high or low.

The trigger of a button push is of course achieved by making a pin go high or low.  But, as @ZeFerby indicates the two buttons associate with a relay can both share the same pin as we just need to know that the pin is triggered and do not care which button (or which panel) triggered it. 

The program needs to be alerted to the fact of a button push and which pin changed its state from high to low (or vice versa) There are several ways to achieve this, but the use of an Interrupt Service Routine is one good way, and is the way I chose for my program.

The wiring of the buttons has to be from its button pin to a shared interrupt pin.  So all buttons are wired to this shared pin and another pin.

In my test program I only allowed for 2 relays.  You will not doubt see how it could easily be expanded to cover more relays without much extra coding.  However you should not really go down this path and should consider multiplexing where just 3 pins can control a raft of LED's.  But as I'm locked down for the corona I enjoyed a couple of hours doing your program my way.  And there will be other ways of course.   At the end of the day you may have to come back to your program and its far better to have one you can read and understand, which may well not be what my code does for you.   However I think my code uses functions for better use of repeating routines.  The debouncing is all done in the interrupt routine.  There is a bunch of Serial.print statements that are not necessary, but used to check the program works without wiring up some real LED's

So here it is.


   
ReplyQuote
(@jbeazy)
Member
Joined: 5 years ago
Posts: 18
 

You have a lot of repeated statements that don't change anything.  For example pins 14 thru 17 only need to be defined as outputs once with your code from pinMode(LED2aPin, OUTPUT); thru  pinMode(LED2dPin, OUTPUT);

The next 12 lines after that can be deleted because they are just restating the same thing over and over.  Same thing with the pinModes: button1aPin and button3aPin are the same integer (6) so it only needs to be defined as INPUT_PULLUP once. And you write a led pin high and then a few lines later write it high again. This is all in the setup. So yes many duplicate lines of code can be eliminated.


   
ReplyQuote
byron
(@byron)
No Title
Joined: 5 years ago
Posts: 1131
 

   
ReplyQuote