Arduino coding question --

Status
Not open for further replies.

robedney

Member
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;

      }
    }
  }
}
 

Attachments

  • read_voltage20.ino
    5.3 KB · Views: 49
Last edited by a moderator:
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.
 
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.
 
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.
 
Status
Not open for further replies.
Back
Top