Go Down

Topic: Limited Programming Experience - Updated (check newest post) (Read 300 times) previous topic - next topic

avluis

#15
Nov 28, 2014, 08:21 pm Last Edit: Dec 03, 2014, 05:59 pm by avluis
Looks a lot longer than mine - or did I omit something?

...R
It just required a few additional lines so that the pins would turn off when level was 0 on an individual side, then separating the sides themselves. That got it working across both sides.
I sort of preferred it that way (where I could tell what side belong to what) and just went with that.

I'm planning on simplifying this function in the future since it can be written in a much simpler way (driver side = 0, any time heatLevel is > 2, side = 1 (passenger side)) but I need to modify a few other functions to account for that.
That should reduce the overall size even further but it won't be as simple as it is now.

Robin2

It just required a few additional lines so that the pins would turn off when level was 0 on an individual side,
I didn't know that level was intended to select a PIN.
I still think it could be much shorter than your version - but if it works, it works.
    If it's not broke don't fix it  :)

...R

LarryD

Quote
If it's not broke don't fix it
If it's not broke, take it apart and see if you can modify it so it works better :smiley-draw:
The way you have it in your schematic isn't the same as how you have it wired up!

avluis

#18
Nov 28, 2014, 10:53 pm Last Edit: Dec 03, 2014, 06:00 pm by avluis
I didn't know that level was intended to select a PIN.
I still think it could be much shorter than your version - but if it works, it works.
    If it's not broke don't fix it  :)

...R

If it's not broke, take it apart and see if you can modify it so it works better :smiley-draw:
While both of these statements are true (in an everyday situation), I would say when it comes to programming to always have a mindset of improvement.
To take the time required to either improve my current programming skills (techniques) but to not shoot too far out there and get stuck guessing.

For now, this code can be considered prototype, as it works beautifully at the moment.
I do have to go over several parts of it with the serial debugger and make sure it is not stepping out of line nor working harder than it should - but at the moment, it does the job and it does it quite well.

And yes, as I learn to program better, gain experience with more coding and research, coming back to prior code and improving it has never hurt.

And that, I look forward to.

LarryD

Yes, I often go back an see if I can improve, document and rearrange things.
The way you have it in your schematic isn't the same as how you have it wired up!

avluis

Hello again!

I'm still waiting on my PCB to make it back, so I have been studying/improving my programming and put some time towards improving my code.
Here is what I have so far (most of the changes are towards the EOD):

Code: [Select]

/*
  Heated Seat Switching
 
  This sketch handles the logic required to control a 4-stage heated seat module.
  The stages are HIGH, MEDIUM, LOW and OFF. OFF is the default-start state.
  Indication of stages is done via LEDs for HIGH, MEDIUM and LOW.
  Vehicle wiring is ground based and built-in LEDs turn on when grounded.
  Control module is positive side switching (Rostra Dual Heated Seats Kit).
 
  Grounding of LEDs is handled by a ULN2003A.
  Level switching is handled by a  M54564P.
  M54564P input pins can be driven by the ULN2003A output pins.
 
  INPUT:
  Pin 2 & 3
 
  OUTPUT:
  Pin 4, 5 and 6 for Driver side.
  Pin 7, 8 and 9 for Passenger side.
*/

// Dashboard Buttons to monitor
const byte buttonPin[] = {2, 3};

// Output to ULN2003A
const byte statusPin[] = {4, 5, 6, 7, 8, 9};

// On-Board LED (on Arduino)
const byte onBoardLedPin = 13;

// Push Counters/State Change
byte buttonPushCounter[] = {0, 0};
byte buttonState[] = {0, 0};
byte lastButtonState[] = {0, 0};

// Timers
unsigned long lastDebounceTime = 0;
// Adjust this if getting button bounce
unsigned long debounceDelay = 6;

// Called when sketch starts
void setup() {
  // Initializing On-Board LED as an output
  pinMode(onBoardLedPin, OUTPUT);
  // Initializing Buttons as inputs
  for (byte x = 0; x < 2; x++){
  pinMode(buttonPin[x], INPUT);
  }
  // Initializing Status Pins as outputs
  for (byte x = 0; x < 6; x++){
  pinMode(statusPin[x], OUTPUT);
  }
}

void loop() {
  // Lets read the state of each button
  queryButtonState();
  // Reset counters if above a set limit
  resetPushCounter();
  // Now we can get some heat going
  toggleHeat();
}

// If the number of button presses reaches
// a certain number, reset press count to 0
void resetPushCounter(){
  for (byte x = 0; x < 2; x++){
    if (buttonPushCounter[x] == 4){
      buttonPushCounter[x] = 0;
    }
  }
}

// Listens for button presses, while debouncing the button input
// and tracks the number of presses respective to each button (side)
void queryButtonState(){
  for (byte x = 0; x < 2; x++){
    buttonState[x] = digitalRead(buttonPin[x]);
    if ((millis() - lastDebounceTime) > debounceDelay){
      if (buttonState[x] != lastButtonState[x]){
          if (buttonState[x] == HIGH && buttonPin[x] == 2){
            buttonPushCounter[0]++;
          }
          if (buttonState[x] == HIGH && buttonPin[x] == 3){
            buttonPushCounter[1]++;
          }
        }
      lastButtonState[x] = buttonState[x];
      lastDebounceTime = millis();
    }
  }
}

// Toggles heat on in accordance to the number of button presses
// on each respective side/button.
// It also toggles heat off if the number of presses loops to 0.
void toggleHeat(){
  if (buttonPushCounter[0] != 0 || buttonPushCounter[1] != 0){
    power(1);
  }
  else{
    power(0);
  }
}

// Toggles the On-Board LED and calls enableHeat()/disableHeat()
// according to power state.
void power(boolean state){
  if (state){
    digitalWrite(onBoardLedPin, HIGH);
    enableHeat();
  } else {
    disableHeat();
    digitalWrite(onBoardLedPin, LOW);
  }
}

// Disables heat when called
void disableHeat(){
  for (byte x = 0; x < 6; x++){
  digitalWrite(statusPin[x], LOW);
  }
}

// Enables heat when called,
// then sends heat level and side to heatLevel()
void enableHeat(){
  for (byte x = 0; x < 2; x++){
    heatLevel(buttonPushCounter[x], x);
  }
}

// This functions receives heat level and heat side arguments,
// it then uses this data to turn off/on the respective pin(s)
void heatLevel(byte level, byte side){
  if (level != 0){
    for (byte n = 0; n < 3; n++){
      digitalWrite(statusPin[n], LOW);
      if (side == 0 && level > 0){
        digitalWrite(statusPin[level] - 1, HIGH);
      }
    }
    for (byte n = 0; n < 3; n++){
      digitalWrite(statusPin[n] + 3, LOW);
      if (side == 1 && level > 0){
        digitalWrite(statusPin[level] + 2, HIGH);
      }
    }
  } else {
    for (byte n = 0; n < 3; n++){
    digitalWrite(statusPin[n], LOW);
    }
  }
}


Not too bad right?
Already spent a good bit of time debugging and I have left it running to observe any weird or out of norm behavior (all good so far).
I hope my code is solid, but I will keep on studying (been reading a book; "Beginning C for Arduino") and improving my code.

It does not hurt to learn and improve the ways we program.

Edit:

This version of the heatLevel() function will work just as well:

Code: [Select]

void heatLevel(byte level, byte side){
  if (level != 0){
    for (byte n = 0; n < 3; n++){
      digitalWrite(statusPin[n], LOW);
      digitalWrite(statusPin[n] + 3, LOW);
      if (side == 0 && level > 0){
        digitalWrite(statusPin[level] - 1, HIGH);
      }
      if (side == 1 && level > 0){
        digitalWrite(statusPin[level] + 2, HIGH);
      }
    }
  } else {
    for (byte n = 0; n < 3; n++){
    digitalWrite(statusPin[n], LOW);
    }
  }
}


I do wonder which one is easier to read, but it is one less for loop for the Arduino to go over.
But because of the way that loop functions, now I can write it like this instead:

Code: [Select]

void heatLevel(byte level, byte side){
  if (level != 0){
    for (byte n = 0; n < 3; n++){
      digitalWrite(statusPin[n], LOW);
      digitalWrite(statusPin[n] + 3, LOW);
      if (side == 0 && level > 0){
        digitalWrite(statusPin[level] - 1, HIGH);
      }
      if (side == 1 && level > 0){
        digitalWrite(statusPin[level] + 2, HIGH);
      }
    }
  }
}


And it will still work as intended, in contrast to the original and much longer loop that required the else statement to turn the pins back off.
This one is essentially turning the pins off first, then turns them on if the condition is true.
And now there is one less for loop for the Arduino to go over.
Guess I am really optimizing at this point (not that it was really required, but it never hurts).

avluis

I haven't stopped playing with my sketch (that PCB is surely taking its time).

I need to implement (or update QueryButtonState()) a way to handle press+hold to be able to trigger a timer.
This will then turn on heat on high for 20 minutes, then down to medium for 10 and settle on low until turned off.

But for now, I have been doing plenty of debugging, and this is what my code looks like now:

Code: [Select]

/*
  Heated Seat Switching
  
  This sketch handles the logic required to control a 4-stage heated seat module.
  The stages are HIGH, MEDIUM, LOW and OFF. OFF is the default-start state.
  Indication of stages is done via LEDs for HIGH, MEDIUM and LOW.
  Vehicle wiring is ground based and built-in LEDs turn on when grounded.
  Control module is positive side switching (Rostra Dual Heated Seats Kit).
  
  Grounding of LEDs is handled by a ULN2003A.
  Level switching is handled by a  M54564P.
  M54564P input pins can be driven by the ULN2003A output pins.
  
  INPUT:
  Pin 2 & 3
  
  OUTPUT:
  Pin 4, 5 and 6 for Driver side.
  Pin 7, 8 and 9 for Passenger side.
*/

// Dashboard Buttons to monitor
const byte buttonPin[] = {2, 3};

// Output to ULN2003A
const byte statusPin[] = {4, 5, 6, 7, 8, 9};

// On-Board LED (on Arduino)
const byte onBoardLedPin = 13;

// Count of button presses for each particular side
byte buttonPushCounter[] = {0, 0};
// Tracks current button state for each particular side
byte buttonState[] = {0, 0};
// Tracks previous button state for each particular side
byte lastButtonState[] = {0, 0};

// Debounce period to filter out button bounce
const byte debounceDelay = 5;
// Last debounce time
unsigned long lastDebounceTime = 0;

// Called when sketch starts
void setup() {
  // Initializing On-Board LED as an output
  pinMode(onBoardLedPin, OUTPUT);
  // Initializing Buttons as inputs
  for (byte x = 0; x < 2; x++){
  pinMode(buttonPin[x], INPUT);
  }
  // Initializing Status Pins as outputs
  for (byte x = 0; x < 6; x++){
  pinMode(statusPin[x], OUTPUT);
  }
}

void loop() {
  // Lets read the state of each button
  QueryButtonState();
  // Reset counters if above a set limit
  ResetPushCounter();
  // Now we can get some heat going
  TogglePower();
}

// If the number of button presses reaches
// a certain number, reset press count to 0
void ResetPushCounter(){
  for (byte x = 0; x < 2; x++){
    if (buttonPushCounter[x] == 4){
      buttonPushCounter[x] = 0;
    }
  }
}

// Listens for button presses, while debouncing the button input
// and tracks the number of presses respective to each button (side)
void QueryButtonState(){
  for (byte x = 0; x < 2; x++){
    noInterrupts();
    buttonState[x] = digitalRead(buttonPin[x]);
    if ((millis() - lastDebounceTime) > debounceDelay){
      if (buttonState[x] == HIGH && lastButtonState[x] == LOW){
          if (buttonPin[x] == 2){
            buttonPushCounter[0]++;
          }
          if (buttonPin[x] == 3){
            buttonPushCounter[1]++;
          }
        }
      lastButtonState[x] = buttonState[x];
      lastDebounceTime = millis();
    }
    interrupts();
  }
}

// Toggles heat ON in accordance to the number of button presses
// on each respective side/button.
// Toggles heat off if the number of presses loops to 0.
void TogglePower(){
  if (buttonPushCounter[0] != 0 || buttonPushCounter[1] != 0){
    Power(1);
  } else {
    Power(0);
  }
}

// Toggles the On-Board LED and calls enableHeat()/disableHeat()
// according to power state.
void Power(boolean state){
  if (state){
    digitalWrite(onBoardLedPin, HIGH);
    ToggleHeat(1);
  } else {
    ToggleHeat(0);
    digitalWrite(onBoardLedPin, LOW);
  }
}

// Enables/Disables heat when called.
// Sends heat level and side to heatLevel() if heat is enabled.
void ToggleHeat(boolean state){
  if (state){
    for (byte x = 0; x < 2; x++){
      HeatLevel(buttonPushCounter[x], x);
    }
  } else {
    for (byte x = 0; x < 6; x++){
      digitalWrite(statusPin[x], LOW);
    }
  }
}

// Receives heat level and heat side arguments.
// Uses this data to turn off/on the respective pin(s)
void HeatLevel(byte level, byte side){
  if (level != 0){
    for (byte n = 0; n < 3; n++){
      digitalWrite(statusPin[n], LOW);
      digitalWrite(statusPin[n] + 3, LOW);
      if (side == 0 && level > 0){
        digitalWrite(statusPin[level] - 1, HIGH);
      }
      if (side == 1 && level > 0){
        digitalWrite(statusPin[level] + 2, HIGH);
      }
    }
  }
}


I'm not 100% convinced on the interrupts, but it keeps that part working properly.
I guess we will see once the arduino is running a timer along with the other functions.

At the same time, once I have the timer function working, there will be more than one instance where the same code will be getting reused.
That means it will be time to use C++ classes & constructors and then just making an instance of it.

Guess it is time to look at the sketch as a whole and see what can be rewritten to work as needed.

See you guys in a few days~

Go Up