Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 3 of 3

Thread: Advice on code design needed

  1. #1

    Advice on code design needed

    Hi - I have been playing with Arduino's and Teensy's for years now but never actually built a project from idea to design to build. I am building a model railway controller for my father and was hoping to get some advice before I go much further.

    At present I am testing on an Arduino Uno and Teensy++ 2.0 but this project will port to a Teensy 3.6 (to be purchased) as soon as I am confident that what I am trying to do will work !

    I have the PWM speed controllers working with thanks to Paul for some non-linear algorithm and array pointer assistance and I am now working on prototyping the button / LED control software.

    I decided (rightly or wrongly!!) to implement custom types as structs to hold the metadata / state information about each button / LED as I thought it would be helpful at a later stage to be able to use that information in code if I had to. There will be around 50 buttons / LED's to control and I still have to prototype an I2C port-expander such as the MCP23017. This is another reason I don't want to use interrupts for button inputs as I don't know how the MCP23017 works with the MCU interrupt handlers but I suppose I could use the MCP23017 to handle all the outputs to free up pins to use for input interrupts if my idea below doesn't work.

    Tonight I did a first cut on some code to define the button / LED structs and iterate over them to setup each button and to then read each button and set LED state on each iteration of the main loop.

    I have tested this and it all works but I am wondering if this is a sensible way to do things. I debounce the switches using a hardware design (RC circuit feeding a 74HC14 Schmitt inverter) so that I can avoid the timing overhead of doing this in code. Hopefully (again, I am not a software engineer nor electronic engineer ) this means I can avoid using interrupts. I figure that given the Teensy 3.6 runs at 180MHz I can harness the extra horsepower to check button states on each loop.

    I have included the code below (no refactoring yet) as I hoped someone might tell me I'm on the right track or if it is utterly stupid to do it the way I am !!

    I have tested it and it all works fine ! Both buttons defined as momentary and latching work as expected and their corresponding LED's light as designed.


    train_controller.ino

    Code:
    #include "MyTypes.h"
    
    // define array lengths
    #define button_count 2
    #define led_count 2
    
    //BUTTONS
    //create button structs
    button_t button0;
    button_t button1;
    //array of pointers
    button_t* buttons[button_count]={
      &button0,
      &button1,
    };
    
    //LEDS
    //create led structs
    led_t led0;
    led_t led1;
    //array of pointers
    led_t* leds[led_count]={
      &led0,
      &led1,
    };
    
    void setup() {
      Serial.begin(9600);
    
      //button setup
      button0.id = 0;
      button0.pin = 11;
      button0.has_led = true;
      button0.led_id = 0;
      //button0.latching = true; //override default
    
      button1.id = 1;
      button1.pin = 4;
      button1.has_led = true;
      button1.led_id = 1;
      
      //led setup
      led0.id = 0;
      led0.pin = 10;
      led1.id = 0;
      led1.pin = 3;
    
      //set up all LEDs
      for (int i = 0; i < led_count; i++) {
        pinMode(leds[i]->pin, OUTPUT);
        //set all LED pins LOW
        digitalWrite(leds[i]->pin, LOW);
      }
      //set up all buttons
      for (int i = 0; i < button_count; i++) {
        pinMode(buttons[i]->pin, INPUT_PULLUP);
      }
    }
    
    void loop() {
      //read all buttons; set states
      for (int i = 0; i < button_count; i++) {
        byte val = digitalRead(buttons[i]->pin);
        buttons[i]->last_state = buttons[i]->state;
        buttons[i]->state = val;
        
        //momentary button going HIGH; turn on LEDs as required
        if (buttons[i]->state == HIGH && buttons[i]->last_state == LOW) {
          Serial.print("BTN");
          Serial.print(buttons[i]->id);
          Serial.print(" MOMENTARY_STATE ");
          Serial.println(buttons[i]->state);
          //assign LED state to non-latching buttons
          if (buttons[i]->has_led == true && buttons[i]->latching == false){
            //set the led struct state to match button state
            leds[buttons[i]->led_id]->state = HIGH;
            Serial.print("LED");
            Serial.print(leds[buttons[i]->led_id]->id);
            Serial.print(" LED_STATE ");
            Serial.println(leds[buttons[i]->led_id]->state);
          }
          
          //latching buttons triggered only when button goes LOW to HIGH
          if (buttons[i]->latching) {
            buttons[i]->latch_state = !buttons[i]->latch_state;
            Serial.print("BTN");
            Serial.print(buttons[i]->id);
            Serial.print(" LATCH_STATE ");
            Serial.println(buttons[i]->latch_state);
            if (buttons[i]->has_led == true){
              //set the led struct state to match button state
              leds[buttons[i]->led_id]->state = buttons[i]->latch_state;
              Serial.print("LED");
              Serial.print(leds[buttons[i]->led_id]->id);
              Serial.print(" LED_STATE ");
              Serial.println(leds[buttons[i]->led_id]->state);
            }   
          }
    
          //do things for individual buttons here
          switch (buttons[i]->id) { //button ID
            case 0:
              //do something for button0
              break;
            case 1:
              //do something for button1
              break;
            default: 
              // if nothing else matches, do the default
            break;
          }
          
        } else {
          //momentary button going LOW; turn off LEDs as required
          if (buttons[i]->state == LOW && buttons[i]->last_state == HIGH) {
            Serial.print("BTN");
            Serial.print(buttons[i]->id);
            Serial.print(" MOMENTARY_STATE ");
            Serial.println(buttons[i]->state);
            //assign LED state to non-latching buttons
            if (buttons[i]->has_led == true && buttons[i]->latching == false){
              //set the led struct state to match button state
              leds[buttons[i]->led_id]->state = LOW;
              Serial.print("LED");
              Serial.print(leds[buttons[i]->led_id]->id);
              Serial.print(" LED_STATE ");
              Serial.println(leds[buttons[i]->led_id]->state);
            }        
          }
        }
      }
    
      //set all LED states
      for (int i = 0; i < led_count; i++) {
        digitalWrite(leds[i]->pin, leds[i]->state);
      }
    
      //speed controller code here
      
    }
    MyTypes.h

    Code:
    #ifndef MyTypes_h
    #define MyTypes_h
    
    typedef struct {
      byte id;
      bool latching = false;
      byte state = LOW;
      byte last_state = LOW;
      byte latch_state = LOW;
      bool has_led = false;
      byte led_id; //led array element; store 256 LED id's
      byte pin; //store 256 pins
    } button_t;
    
    typedef struct {
      byte id;
      byte state = LOW;
      byte pin;
    } led_t;
    
    #endif
    Last edited by ilium007; 10-23-2017 at 07:06 AM.

  2. #2
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    25,227
    Looks reasonable to me.

    These sorts of data structures and abstraction layers are mostly a matter of personal taste. Some people put a ton of work (complexity) into them, to the point I wonder if their life would be simpler just copying and pasting the code.

    However, assigning values within a typedef might not be the best way? Usually when people do this sort of thing, they turn it into a C++ object and use initializers or a constructor to set up the values of member variables. Some people like to get into C++ templates, though I personally avoid them in most cases. Opinions quite a lot of these matters....

  3. #3
    Thanks for the reply ! I'll forge ahead as I know no better ! Worst case I can grab a Teensy LC to run the PWM controllers and the 3.6 to do the heavy lifting due to my inefficient code.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •