Pages: 1 [2] 3   Go Down
Author Topic: Help needed to convert some code.  (Read 2892 times)
0 Members and 1 Guest are viewing this topic.
0
Offline Offline
Newbie
*
Karma: 0
Posts: 24
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Not every if statement needs an else. If the else is not doing anything, get rid of it.

These do. If you read through the function's logic you'll see that they are all dependent on one another. There was yet another if...else in there, but I managed to refactor them into one. Unfortunately, that was the only one I have been able to refactor without breaking the logic. If you can find a way to reorganise it and have it perform the same function, I would appreciate the help.

Quote
The else; look like errors. At least, errors waiting to happen.

The original code compiles with no errors, so I transposed it almost verbatim, editing for the differences in syntax between C and Arduino. If the Arduino compiler throws a hissy fit, it's due to its interpretation of the C language, not my initial code.
Quote
It's an array, not a function.

Good catch, thanks.

Quote
Not much point initiailising the flag, if you're going to overwrite it:

Unless of course, you need it resetting at the beginning of each loop.
« Last Edit: April 16, 2010, 09:34:20 am by flapjackboy » Logged

UK
Offline Offline
Faraday Member
**
Karma: 17
Posts: 2884
Gorm deficient
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The "else;"s do nothing - the fact that there's a semicolon after them signals that.
They are redundant, as is the "return;".

Quote
Unless of course, you need it resetting at the beginning of each loop.
But you overwrite it at the beginning of every loop!

Just because a bit of code compiles without errors doesn't mean it doesn't harbour potential nasties.
Most languages allow you to shoot yourself in the foot, but C hands you the gun.
If you cut out redundant code, you leave fewer dark corners for the bugs to hide.
« Last Edit: April 16, 2010, 09:41:17 am by GrooveFlotilla » Logged

Per Arduino ad Astra

0
Offline Offline
Newbie
*
Karma: 0
Posts: 24
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
The "else;"s do nothing - the fact that there's a semicolon after them signals that.
They are redundant, as is the "return;".

So they are, just test compiled the original code without them and it returned no errors.

Quote
But you overwrite it at the beginning of every loop!

D'oh!

Just goes to show how useful it is having an objective pair of eyes go over your code, lol.
« Last Edit: April 16, 2010, 09:44:38 am by flapjackboy » Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 24
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
void CheckInputs(boolean screen = true);  //passes a true value to screen
                                          //and sets CheckInputs to perform
                                          //the initial input check.

boolean inputflag = false;                //the initial input error check
boolean exitflag = false;
int x;
const int buttonPins[] = {2, 3, 4, 5, 6};

void setup()
{
    Serial.begin(9600);
}

void loop()
{
  
  CheckInputs();
  
  Serial.println("Initial Check Complete");
  Serial.println("Enter number of questions");
  x = Serial.read();
  
  for (int i = 0; i < x; i++)
  {
    do
    {
      CheckInputs(false);
      
      if (digitalRead(buttonPins[6]));
      {
        for (int a= 2; a <=6; a++)
          {
            digitalWrite(buttonPins[a], LOW);
          }
        inputflag = false;
        exitflag = true;
      } while (exitflag == false);
      
    while (digitalRead(buttonPins[6]));
      
    exitflag = false;
    }
    
    
  }
)

void CheckInputs(boolean screen)
{
  int i;
  for (i = 2; i <=6; i++)
  {
    boolean inputstatus = digitalRead(buttonPins[i]);
    
    if (inputstatus)
    {
      Serial.println("Input " i " depressed");
      inputflag = true;
    }
    else
    {
      if (i != 6)
        if (!inputflag)
        {
          Serial.println("Contestant " i-1 " buzzed!");
          inputflag = true;
        }
    }
  }
  
  if (!inputflag && screen)
  {
    Serial.println("No inputs depressed");
  }
  
}

OK, so this is how far I've got so far.
« Last Edit: April 16, 2010, 11:17:17 am by flapjackboy » Logged

UK
Offline Offline
God Member
*****
Karma: 0
Posts: 710
Arduino is Genius
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

This doesn't compile, right?

It has errors around lines like:
Code:
Serial.println("Input " i " depressed");
which are not legal C or C++.

This would need to become
Code:
Serial.print("Input ");
Serial.print( i );
Serial.println(" depressed");

Also
Code:
 for (i = 2; i <=6; i++)
  {
    boolean inputstatus = digitalRead(buttonPins[i]);
is likely wrong, because the only valid indexes for buttonPins[] is 0 to 4.

HTH
GB
« Last Edit: April 16, 2010, 11:30:57 am by gbulmer » Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 24
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I think I've got it, got a clean compile with this:

Code:
void CheckInputs(boolean screen = true);  //passes a true value to screen
                                            //and sets CheckInputs to perform
                                            //the initial input check.

boolean inputflag = false;                  //the initial input error check
boolean exitflag = false;
int x;
const int buttonPins[] = {2, 3, 4, 5, 6};

void setup()
{
    Serial.begin(9600);
}

void loop()
{
  
  CheckInputs();
  
  Serial.println("Initial Check Complete");
  Serial.println("Enter number of questions");
  x = Serial.read();
  
  for (int i = 0; i < x; i++)
  {
    do
    {
      CheckInputs(false);

      if (digitalRead(buttonPins[4]));
      {
        for (int a= 0; a <=4; a++)
          {
            digitalWrite(buttonPins[a], LOW);
          }
        inputflag = false;
        exitflag = true;
      } while (exitflag == false);
    }
    while (digitalRead(buttonPins[4]));

    exitflag = false;
    
    
    
  }
}

void CheckInputs(boolean screen)
{
  int i;
  for (i = 0; i <=4; i++)
  {
    boolean inputstatus = digitalRead(buttonPins[i]);
    
    if (inputstatus)
    {
      Serial.print("Input ");
        Serial.print(i+1);
        Serial.println(" depressed");
      inputflag = true;
    }
    else
    {
      if (i != 4)
        if (!inputflag)
        {
          Serial.print("Contestant ");
            Serial.print(i+1);
            Serial.println(" buzzed!");
          inputflag = true;
        }
    }
  }
  
  if (!inputflag && screen)
  {
    Serial.println("No inputs depressed");
  }
  
}

Hopefully, this will work.

EDIT: Nope, it just keeps looping through the initial check saying all inputs are pressed. smiley-sad
« Last Edit: April 16, 2010, 12:23:57 pm by flapjackboy » Logged

Global Moderator
UK
Offline Offline
Brattain Member
*****
Karma: 299
Posts: 26186
I don't think you connected the grounds, Dave.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Nope, it just keeps looping through the initial check saying all inputs are pressed

Pull-ups?
Pull-downs?
Logged

"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

0
Offline Offline
Newbie
*
Karma: 0
Posts: 24
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Well, for the contestant buttons, I'm using some handsets scavenged from an interactive DVD quiz game. These have 5 buttons, each with a different value resistor in series with it. The original control box used the different voltage potentials to distinguish which button had been pressed on each handset.

The only button I could get the Arduino to pick up when I tested it was the one with the 1K resistor.
Logged

Seattle, WA USA
Online Online
Brattain Member
*****
Karma: 610
Posts: 49003
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
     if (digitalRead(buttonPins[4]))[glow];[/glow]

This isn't doing anything...

Code:
       exitflag = true;
      } while (exitflag == false);

Doesn't look like exitFlag will ever get to be false.

Code:
           digitalWrite(buttonPins[a], LOW);

Are the pins listed in buttonPins input or output? You are trying to use them as both.

You can't have it, with switches attached, both ways.
Logged

Global Moderator
UK
Offline Offline
Brattain Member
*****
Karma: 299
Posts: 26186
I don't think you connected the grounds, Dave.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

How are the switches wired?

Probably the simplest way is to use the AVR's internal pull-ups, but this inverts the logic of an open switch (It will be HIGH when open, and LOW when closed), but this is simple to deal with in software.

Without pull-ups or pull-downs, the pins are floating and can appear random.

Have a look at the examples and tutorials.
Logged

"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

0
Offline Offline
Newbie
*
Karma: 0
Posts: 24
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

OK, I think that rather than try to shoehorn the old code into the Arduino, I'm going to have to rewrite this from scratch. stick with the code that works and use the K8055.

Thanks for all the help, but as the old adage goes, if it ain't broke, don't fix it.
« Last Edit: April 16, 2010, 02:27:37 pm by flapjackboy » Logged

Global Moderator
UK
Offline Offline
Brattain Member
*****
Karma: 299
Posts: 26186
I don't think you connected the grounds, Dave.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I think that rather than try to shoehorn the old code into the Arduino

I don't know about the other guys here, but I'm glad I didn't invest too much time in your day.
Logged

"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

UK
Offline Offline
God Member
*****
Karma: 0
Posts: 710
Arduino is Genius
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Quote
I think that rather than try to shoehorn the old code into the Arduino


I don't know about the other guys here, but I'm glad I didn't invest too much time in your day.
I felt a bit annoyed that I'd spent my limited time on someone who gave up, when I might have helped someone who made some progress instead. But I know who I'll avoid helping in future, and ...

I'm now quite pleased that I have an example thread where a simple program was made so complex that it defeated the author who claimed they'd got it working on different hardware.

In future I can reference this thread, and say "Rather than start with that complex old thing, take confidence from it that you can do it again, but instead start with a simple program. You will make progress faster. You might learn something new. It may even turn out much better! Sadly, this person didn't do that, and ended up giving up!"

Many silver linings are embedded in clouds ;D

GB


« Last Edit: April 16, 2010, 03:32:59 pm by gbulmer » Logged

Global Moderator
UK
Offline Offline
Brattain Member
*****
Karma: 299
Posts: 26186
I don't think you connected the grounds, Dave.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

gbulmer, had I a cap to doff, I would doff it at you!

I like your attitude - you are obviously far less cynical than I.   ;D
« Last Edit: April 16, 2010, 02:59:37 pm by AWOL » Logged

"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

UK
Offline Offline
God Member
*****
Karma: 0
Posts: 710
Arduino is Genius
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

AWOL - sorry I missed a bit and modified it.
I may have tarnished my attitude

GB
« Last Edit: April 16, 2010, 03:33:18 pm by gbulmer » Logged

Pages: 1 [2] 3   Go Up
Jump to: