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

Thread: Arduino coding question --

  1. #1
    Junior Member
    Join Date
    Jul 2021
    Posts
    7

    Arduino coding question --

    The last time I coded anything was a billion years ago in Basic. I've been learning Arduino for a project I'm working on using a Teensy, but I'm stuck. The sketch shown here is cobbled together, but works with one exception. Functionally, it reads a key stroke on a typewriter. The individual key bar contacts a resistance strip, and analog 0 senses the voltage. That number is then put through a filter to clean it up (average/median), then drops down through a filter to determine which key was read. The good news is that I've finally got it all working, with one exception that is driving me nuts and I'm too new to this all to debug. When, for example, I press the "A" key it reads the right number range and gives me an A. However, when I move to another key -- say "B" -- the first key press on B also returns an A. Any key press after the first correctly returns a B. This is true anywhere on the keyboard. I understand that the last key press is somehow still being retained in the variable, but I can't figure out where or how to fix it, believe me I've tried. I understand that I don't know enough to be doing this, but I'm almost there Other critiques and suggestions are welcomed!

    I've attached the code as a file, but in case I messed that up here it is:

    Code:
    /*
      The Analog median and average sketch used below is by Tom Igoe
      The program reads an analog input and gives the average of 9 readings,
      and sorts the list of readings and delivers the median number.
    
      The sketch used to read the key tritter switch is from a tutorial on Youtube.
    
      The rest is my own halting, beginner's effort
    
     */
    //my added variables
    const int BUTTON_PIN = 2;       // the number of the pushbutton pin
    int lastState = LOW;  // the previous state from the input pin
    int currentState;                // the current reading from the input pin
    int buttonPress;
    int test_counts;
    //end added variables 
    
    
    int numReadings = 9; // number of samples to take
    int median = 0;      // median of the sorted samples
    int readingNumber;   // counter for the sample array
    int counts = 0;
    
    // variables for subroutines:
    byte i = 0;
    byte j = 0;
    byte position = 0;
    int analogValues[9]; 
    
    //function prototypes:
    void bubbleSort();
    int  averageArray();
    
    void setup() {
      Serial.begin(9600);
      pinMode(BUTTON_PIN, INPUT_PULLUP);
      
    }
    
    void loop() {
      //added button code
     
    READ: 
    currentState = digitalRead(BUTTON_PIN);
     if(lastState == HIGH && currentState == LOW)
    Serial.println();
    else if(lastState == LOW && currentState == HIGH)
    Serial.println();
    
      // save the the last state
      lastState = currentState;
      if(currentState == LOW)
    
      goto READKEYS; 
      else if (currentState == HIGH) goto READ;
      
      
      //end added button code
    
      READKEYS:
      delay(25);
      for (readingNumber = 0; readingNumber < numReadings; readingNumber++) {
        //get the reading:
        analogValues[readingNumber] = analogRead(0);
        // increment the counter:
        readingNumber++;
      }
      // sort the array using a bubble sort:
      bubbleSort();
    
      // get the middle element:
      median = analogValues[numReadings / 2]; 
    
      // print the results:
      // print the array, nicely ASCII-formatted:
     // Serial.print("Array: [");
      for (j = 0; j < numReadings; j++) {
        //Serial.print(analogValues[j], DEC);
       // Serial.print (", ");
      }
     // Serial.print("]\r\n");
      // average the array:
      /*Serial.print(" Average = ");
      Serial.print(averageArray(), DEC);
      Serial.print("\tMedian = ");
      Serial.print(median, DEC);
      Serial.print("\r\n");
    */
    
    
    //Serial.println(counts);
      delay(50);
    }
    
    // average the values in the array:
    int  averageArray() {
      int total = 0;
      int average = 0;
      for (i = 0; i< numReadings; i++) {
        total = total + analogValues[i];
      }
      average = total/(numReadings + 1);
      return average;
    }
    
    void bubbleSort() {
      int out, in, swapper;
      for(out=0 ; out < numReadings; out++) {  // outer loop
        for(in=out; in<(numReadings-1); in++)  {  // inner loop
          if( analogValues[in] > analogValues[in+1] ) {   // out of order?
            // swap them:
            swapper = analogValues[in];
            analogValues [in] = analogValues[in+1];
            analogValues[in+1] = swapper;
    
          
           counts = 0;
           counts = median;
    
    if (counts >887&& counts <907) Serial.println ("a");
    if (counts >552&& counts <572) Serial.println ("b");
    if (counts >690&& counts <710) Serial.println ("c");
    if (counts >720&& counts <740) Serial.println ("d");
    if (counts >741&& counts <761) Serial.println ("e");
    if (counts >650&& counts <670) Serial.println ("f");
    if (counts >588&& counts <608) Serial.println ("g");
    if (counts >518&& counts <538) Serial.println ("h");
    if (counts >400&& counts <420) Serial.println ("i");
    if (counts >450&& counts <470) Serial.println ("j");
    if (counts >378&& counts <398) Serial.println ("k");
    if (counts >274&& counts <294) Serial.println ("l");
    if (counts >423&& counts <443) Serial.println ("m");
    if (counts >490&& counts <510) Serial.println ("n");
    if (counts >310&& counts <330) Serial.println ("o");
    if (counts >210&& counts <230) Serial.println ("p");
    if (counts >912&& counts <932) Serial.println ("q");
    if (counts >673&& counts <693) Serial.println ("r");
    if (counts >800&& counts <820) Serial.println ("s");
    if (counts >602&& counts <622) Serial.println ("t");
    if (counts >467&& counts <487) Serial.println ("u");
    if (counts >623&& counts <643) Serial.println ("v");
    if (counts >825&& counts <845) Serial.println ("w");
    if (counts >718&& counts <738) Serial.println ("x");
    if (counts >537&& counts <557) Serial.println ("y");
    if (counts >852&& counts <872) Serial.println ("z");
    if (counts >-10&& counts <10) Serial.println ("1");
    if (counts >-10&& counts <10) Serial.println ("2");
    if (counts >-10&& counts <10) Serial.println ("3");
    if (counts >-10&& counts <10) Serial.println ("4");
    if (counts >-10&& counts <10) Serial.println ("5");
    if (counts >-10&& counts <10) Serial.println ("6");
    if (counts >-10&& counts <10) Serial.println ("7");
    if (counts >-10&& counts <10) Serial.println ("8");
    if (counts >-10&& counts <10) Serial.println ("9");
    if (counts >-10&& counts <10) Serial.println ("10");
    if (counts >-10&& counts <10) Serial.println ("11");
    if (counts >-10&& counts <10) Serial.println ("12");
    if (counts >-10&& counts <10) Serial.println ("13");
    if (counts >-10&& counts <10) Serial.println ("14");
    if (counts >-10&& counts <10) Serial.println ("15");
    if (counts >-10&& counts <10) Serial.println ("16");
    if (counts >-10&& counts <10) Serial.println ("17");
    Serial.println(counts);
    counts = 0;
    
          }
        }
      }
    }
    Attached Files Attached Files
    Last edited by defragster; 08-11-2021 at 05:23 AM. Reason: add # / CODE blocking for readability

  2. #2
    Senior Member
    Join Date
    Jul 2020
    Posts
    1,224
    One thing I spotted is duplicate incrementing of readingNumber:
    Code:
      for (readingNumber = 0; readingNumber < numReadings; readingNumber++) // <<<<<<<<<<<<<<<<<<<<<<<<<
      {
        //get the reading:
        analogValues[readingNumber] = analogRead(0);
        // increment the counter:
        readingNumber++;  // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
      }
    Your averaging function divides by one more than the element count which can't be right.

    Why are you printing out that stuff in the middle of the bubblesort inner loop? You need to bubblesort, get the median, then print out.

  3. #3
    Senior Member
    Join Date
    May 2015
    Location
    USA
    Posts
    1,070
    To the maximum extent possible, use library routines. Like qsort().

  4. #4
    Senior Member
    Join Date
    Dec 2016
    Location
    Wales
    Posts
    122
    The answer to your main question :

    The code that prints the key stroke is executed in bubbleSort(), line 119 counts is set to the value of median and the key is determined from that.
    bubbleSort() is called at line 70, but the value of median is determined at line 73. Hence bubbleSort is sorting the current data set, but printing based on the last.

    Other issues:

    Your if statements that determine the key are part of the bubbleSort for loops, and a within line 111's if. As far as I can tell, this should result in a print every time the sort swaps two values. You could move these to their own function.

    Those ifs should be if - else statements. There's also a lot of gaps between the thresholds, where I'd argue there shouldn't be. It might be ambiguous whether a reading of 823 is an S or a W, but printing neither is guaranteed to be wrong.

    Why are you using a median and not an average?

    You're using a pretty manual method of debouncing the button from line 45. Look at the bounce library for both a more reliable method, but also a good starting point for using libraries in general.

  5. #5
    Some general comments:

    Code:
    READ: 
      currentState = digitalRead(BUTTON_PIN);
      if(lastState == HIGH && currentState == LOW)
        Serial.println();
      else if(lastState == LOW && currentState == HIGH)
        Serial.println();
    
      // save the the last state
      lastState = currentState;
      if(currentState == LOW)
        goto READKEYS; 
      else if (currentState == HIGH)
        goto READ;
      
    READKEYS:
      // ...
    Don't use goto, use human-readable control structures like if, while, do-while, for, function calls, etc. instead:
    Code:
    do { 
      currentState = digitalRead(BUTTON_PIN);
      if(lastState == HIGH && currentState == LOW)
        Serial.println();
      else if(lastState == LOW && currentState == HIGH)
        Serial.println();
    
      // save the the last state
      lastState = currentState;
    } while (currentState == HIGH);
    
    // ...
    Using this while loop (or goto) here might not be a great idea, it blocks the entire system while waiting for a button press. It might be better to just poll the button input once on every iteration of the loop function. Or better yet, use a library that abstracts all the state change detection and debouncing away from the user.

    void bubbleSort()
    Unless you're doing exercises for an algorithms course or unless you have really specific requirements, don't write your own sorting algorithms (especially not bubble sort). Use the standard library function `std::sort`. Don't use `qsort` unless you're limited to C.

    Code:
    #include <algorithm>
    
      std::sort(std::begin(analogValues), std::end(analogValues));
      // the analogValues array is now sorted, from beginning to end
    In fact, you don't even need to sort the array at all, to calculate the median, you just need the element in the middle, sorting the rest is unnecessary work. Use the `std::nth_element` function:
    Code:
    #include <algorithm>
    
        const int numReadings = 9;
        int analogValues[numReadings] = {9, 4, 1, 5, 7, 6, 3, 2, 8};
        auto median = std::begin(analogValues) + numReadings / 2;
        std::nth_element(std::begin(analogValues), median, std::end(analogValues));
        Serial.print("The median is ");
        Serial.println(*median); // 5
    Going even further, a median filter is usually overkill for filtering an analog input. Averaging (possibly with hysteresis) is usually sufficient.

    int readingNumber; // counter for the sample array
    This should be a local variable inside your loops, don't make it global. Idem for your other loop indices and temporary variables: make them as local as possible.
    Code:
    for (int readingNumber = 0; readingNumber < numReadings; readingNumber++)
    int numReadings = 9; // number of samples to take
    int analogValues[9];
    Don't leave room for these to get out of sync if you change one:
    Code:
    const int numReadings = 9;
    int analogValues[numReadings];
    Code:
    // average the values in the array:
    int  averageArray() {
      int total = 0;
      int average = 0;
      for (i = 0; i< numReadings; i++) {
        total = total + analogValues[i];
      }
      average = total/(numReadings + 1);
      return average;
    }
    This can be a one-liner:
    Code:
    #include <numeric>
    
    // add up all elements in the array and divide by the number of elements
    return std::accumulate(std::begin(analogValues), std::end(analogValues), 0) / numReadings;
    In fact, if you remove the median filter, you can remove the analogValues array entirely and just keep a running sum while you're doing the measurements.

Posting Permissions

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