How would you do this?

Hi, so I'm still very new to the programming side of electronics, and I am currently working on a fix/mod for my BK function generator.

The mechanical switches for wave type selection were damaged, so I've resolved to fix it with a microcontroller. I'm planning to use an attiny85, but so far I'm just using my BBB to test it, and I've run into a problem (I suck at coding.)

const int sinpin = 0;   // Sine Wave button
const int squpin = 1;    // Square Wave button
const int tripin = 2 ;   // Triangle Wave button

const int sine = 12;     // Sine Wave output
const int tri = 13;      // Triangle Wave output

void setup () {
  pinMode(sine, OUTPUT);
  pinMode(tri, OUTPUT);
  pinMode(sinpin, INPUT);
  pinMode(squpin, INPUT);
  pinMode(tripin, INPUT);
}


void Sinstate () { digitalRead(sinpin); }
void Squstate () { digitalRead(squpin); }
void Tristate () { digitalRead(tripin); }
    
    
void loop(){

    pinMode (sinpin, HIGH);        //Sets internal pullups
    pinMode (squpin, HIGH);
    pinMode (tripin, HIGH);
    
  
      if (Sinstate == HIGH) {            //Sine Things
        digitalWrite(sine, LOW);
        digitalWrite(tri, HIGH); }
      else {
        if (Sinstate == LOW) {
          if (Squstate == LOW) {
            digitalWrite(sine, LOW;)
            digitalWrite(tri, HIGH); }
          if (Tristate == LOW) {
            digitalWrite(sine, LOW);
            digitalWrite(tri, HIGH); } } 
           }

            
        
      if (Squstate == HIGH) {            //Square Things
        digitalWrite(sine, HIGH);
        digitalWrite(tri, HIGH); }
      else {
        if (Squstate == LOW) {
          if (Sinstate == LOW) {
            digitalWrite(sine, HIGH);
            digitalWrite(tri, HIGH); }
          if (Tristate == LOW) {
            digitalWrite(sine, HIGH);
            digitalWrite(tri, HIGH); } }
          }
        
        
      if (Tristate == HIGH) {            //Triangle Things
         digitalWrite(sine, HIGH);
        digitalWrite(tri, LOW); }
      else {
        if (Tristate == LOW) {
          if (Squstate == LOW) {
            digitalWrite(sine, HIGH);
            digitalWrite(tri, LOW); }
          if (Sinstate == LOW) {
            digitalWrite(sine, HIGH);
            digitalWrite(tri, LOW); } }
           }
      }

The intent of the program is to take an input from one of three push button switches (one for each selectable wave type) to set the state of the pins marked "Sine" and "Tri." I am unsure of how to do this more simply (though I'm hoping its not too far above my capabilities). The arduino is meant to send the signals through a transistor to ground the two controlling wires in the function generator.

The above code is currently giving me these errors when I attempt to compile it.

sketch_jun09a.cpp: In function 'void loop()':
sketch_jun09a:28: error: ISO C++ forbids comparison between pointer and integer
sketch_jun09a:34: error: expected `)' before ';' token
sketch_jun09a:34: error: expected primary-expression before ')' token
sketch_jun09a:34: error: expected `;' before ')' token
sketch_jun09a:43: error: ISO C++ forbids comparison between pointer and integer
sketch_jun09a:57: error: ISO C++ forbids comparison between pointer and integer

If you would like it, here is the table for the output selection via grounding on the BK generator. It has internal pullups (on the machine, not affecting the arduino), with low being, of course, when they are grounded.

                         Control 1              Control 2
Sine Wave                 High                   Low
Square Wave               Low                    Low
Triangle/Sawtooth         Low                    High

If you wouldn't mind helping, it would be great!
Thanks.

And sorry for double posting, but please don't mind the silly semicolon errors, I completely understand those XD

void Sinstate () { digitalRead(sinpin); }
void Squstate () { digitalRead(squpin); }
void Tristate () { digitalRead(tripin); }

You are defining 3 functions here which do a digital read and discard the result.

      if (Sinstate == HIGH) {            //Sine Things

Here you are taking the function and comparing it to HIGH. This is the actual function you are comparing, not the results of calling the function.

You should have:

int Sinstate () { return digitalRead(sinpin); }
int Squstate () { return digitalRead(squpin); }
int Tristate () { return digitalRead(tripin); }

and then use the functions like this:

      if (Sinstate() == HIGH) {            //Sine Things

Note the () after the name of the function. This denotes calling the function, not just the address the function resides at in memory.

    pinMode (sinpin, HIGH);        //Sets internal pullups
    pinMode (squpin, HIGH);
    pinMode (tripin, HIGH);

No. The pinMode() function does not set the internal pullups. Why would you want to do that in loop(), anyway?

      if (Sinstate == HIGH) {            //Sine Things

Sinstate is the address of a function. It will never be HIGH.

        digitalWrite(tri, HIGH); }

There are a number of coding styles that define where the { goes, and whether it is indented or not. None of them suggest that the } goes on the same line as any other code.

I prefer the style that puts the { on a new line. To me, it makes the block of code more apparent.

The pinMode() function does not set the internal pullups.

It does in 1.0.1, but not like that.

As of Arduino 1.0.1, it is possible to enable the internal pullup resistors with the mode INPUT_PULLUP. Additionally, the INPUT mode explicitly disables the internal pullups.

Thanks! So I've fixed my code with the modifications that you all suggested.

Unfortunately I still have a problem, from what I can see, all of the if/else functions(?) are conflicting with one another, and that is pushing my output pins to a state that's not quite high, but it's also not quite low (I'm using LED's to test it). However, I can still see which pins are going higher and lower with the inputs, so at least some part of it is working.

I was thinking about using a counter type of function to keep track of which button was selected last, but I think that this could come up with errors after multiple presses. So, if you can suggest an effective way to store the last pressed button please share!

Thank you again.

So I've fixed my code with the modifications that you all suggested.

But, you're not going to show it to us, but you still want help. I'll leave it to you to imagine the amount of help you'll get without, or with, the code.

and that is pushing my output pins to a state that's not quite high, but it's also not quite low

Not physically possible. The outputs may not be remaining at the state you want long enough, but there is only HIGH or LOW. Nothing in between (for digital pins).

So, if you can suggest an effective way to store the last pressed button please share!

Trivially easy, or just about impossible, depending on YOUR code. (See above).

that is pushing my output pins to a state that's not quite high,

Not possible unless there is way too much load on the pins, in which case you are damaging the Arduino.

I'm using LED's to test it

Looking at a LED is not a test, it could be doing anything at 10000Hz (or whatever) and you wouldn't know.


Rob

Writing a HIGH to a pin set as INPUT enables the pull-up resistors.

This can seem, when examining the pin, as if the pin is an output but is "almost" high.

Make sure the pin modes are right for your pins.

Sorry I forgot to post the code.

const int sinpin = 2;   // Sine Wave button
const int squpin = 3;    // Square Wave button
const int tripin = 4 ;   // Triangle Wave button

const int sine = 12;     // Sine Wave output
const int tri = 13;      // Triangle Wave output

void setup () {
  pinMode(sine, OUTPUT);
  pinMode(tri, OUTPUT);
  pinMode(sinpin, INPUT_PULLUP);    //Sets Internal Pullups
  pinMode(squpin, INPUT_PULLUP);
  pinMode(tripin, INPUT_PULLUP);
}

int Sinstate () { return digitalRead(sinpin); }
int Squstate () { return digitalRead(squpin); }
int Tristate () { return digitalRead(tripin); }
    
    
void loop(){
  
      if (Sinstate() == LOW) {            //Sine Things
        digitalWrite(sine, LOW);
        digitalWrite(tri, HIGH); }
      else {
        if (Sinstate() == HIGH) {
          if (Squstate() == LOW) {
            digitalWrite(sine, LOW);
            digitalWrite(tri, HIGH); }
          if (Tristate() == HIGH) {
            digitalWrite(sine, LOW);
            digitalWrite(tri, HIGH); }
             } 
           }

            
        
      if (Squstate() == LOW) {            //Square Things
        digitalWrite(sine, HIGH);
        digitalWrite(tri, HIGH); }
      else {
        if (Squstate() == HIGH) {
          if (Sinstate() == HIGH) {
            digitalWrite(sine, HIGH);
            digitalWrite(tri, HIGH); }
          if (Tristate() == HIGH) {
            digitalWrite(sine, HIGH);
            digitalWrite(tri, HIGH); }
            }
          }
        
        
      if (Tristate() == LOW) {            //Triangle Things
         digitalWrite(sine, HIGH);
         digitalWrite(tri, LOW); }
      else {
        if (Tristate() == HIGH) {
          if (Squstate() == HIGH) {
            digitalWrite(sine, HIGH);
            digitalWrite(tri, LOW); }
          if (Sinstate() == HIGH) {
            digitalWrite(sine, HIGH);
            digitalWrite(tri, LOW); }
            }
          }
}

Graynomad:
Looking at a LED is not a test, it could be doing anything at 10000Hz (or whatever) and you wouldn't know.


Rob

I just checked on an oscilloscope since you mentioned it, and the two output pins are oscillating at 10.53khz and getting faster and slower when I hit the input buttons!

Dang I'm good, .53kHz off :slight_smile:


Rob

      if (Sinstate() == LOW) {            //Sine Things
                         }
      else {
        if (Sinstate() == HIGH) {

If the sin pin isn't LOW, there is no possible value other than HIGH. It is useless to test for it to then be HIGH. It MUST be, if it isn't LOW.

Still no Serial.print() statements, to validate that the pin states are being read correctly. Why not? It almost seems like you are not interested in helping solve YOUR problem.

PaulS. I know creating print statements for debugging is obvious to you. However, to somebody who is new to programming it is not.

Legot:
I'm still very new to the programming side

The tone of last sentence in your post makes complete sense to you, who have spent a large amount of time on this forum (helping people), but not to the OP, who is new here.

The tone of last sentence in your post makes complete sense to you, who have spent a large amount of time on this forum (helping people), but not to the OP, who is new here.

"But, I don't know how" is a perfectly acceptable response. If that response comes, I'm perfectly happy to explain how and where to put all the Serial method calls to show what the program is doing.

Personally, I get a far bigger thrill out of solving problems myself than from asking someone else to help. Sometimes that is not always possible, so I need a nudge in the right direction. I do not, however, need to be taken by the hand and shown the complete way.

Knowing HOW to debug a program is a very important skill. That skill should be acquired as soon as possible.

Ok, so after leaving it alone for a couple of days, I came back and rethought it today. It works!
Thanks for all of your help, and sorry for being such a noob. It've learned alot from the research your comments made me do.

const int sinpin = 2;   // Sine Wave button
const int squpin = 3;    // Square Wave button
const int tripin = 4 ;   // Triangle Wave button

const int sine = 12;     // Sine Wave output
const int tri = 13;      // Triangle Wave output

int Sinstate () { return digitalRead(sinpin); }
int Squstate () { return digitalRead(squpin); }
int Tristate () { return digitalRead(tripin); }
    
int SinActive () { return digitalRead(sine); }
int TriActive () { return digitalRead(tri); }    
    
    //Truth Table
        //Sine Wave
          //sine = LOW
          //tri = HIGH
        //Square Wave
          //sine = HIGH
          //tri = HIGH
        //Triangle wave
          //sine = HIGH
          //tri = LOW

void setup () {
  Serial.begin(9600);
  pinMode(sine, OUTPUT);
  pinMode(tri, OUTPUT);
  pinMode(sinpin, INPUT_PULLUP);    //Sets Internal Pullups
  pinMode(squpin, INPUT_PULLUP);
  pinMode(tripin, INPUT_PULLUP);
}

void loop(){
    if (Sinstate() == LOW) {
      digitalWrite (sine, LOW);
      digitalWrite (tri, HIGH);
    }

    if (Squstate() == LOW) {
      digitalWrite (sine, HIGH);
      digitalWrite (tri, HIGH);    
    }   
            
    if (Tristate() == LOW) {
       digitalWrite (sine, HIGH);
       digitalWrite (tri, LOW);
    }



    if (SinActive() == LOW) {                           //Tests! Comment out later
              Serial.println("--Sin-High");
               }
             else {
               Serial.println("--Sin-Low");
               }
               
            if (TriActive() == LOW) {
              Serial.println("  Tri-High");
              Serial.println(" ");
               }
             else {
               Serial.println("  Tri-Low");
               Serial.println(" ");
             }
               delay(1000);                    //makes it readable

           
}

Thanks!