Jump to content

Arduino problems with code

SDWdidi

Hello,

I am currently doing a minin project for schoolwhere I have to make an elevator with my arduino.

Now, the problem is that I have 2 if commands that work under similar conditions which makes them conflict that code would be the following (check in the quote, the problem is that the arduino will not go activate the buzzard as coded in the last if statement).

 

Quote

int buzz = 2;
int peso = A0;
int mot = 3; //En este momento el contactor para arduino representa el motor.
// aca abajo van todos los botones que estan adentro del elevador
int s1 = 13;
int s2 = 12;
int s3 = 11;
//Debajo de esto van todos los finales de carrera
int fc1 = 10;
int fc2 = 9;
int fc3 = 8;
// debajo de esto estan los botones de llamado de cada piso

//debajo de esto va la parada de mantenimiento
int mant = 4;
int led = 7;

// "peso" es el potenciometreo que "sensa" el peso. En el momento que se pasa de cirto valor (en este caso 900). sonara el Buzzer (buzz).

void setup() {
  pinMode(buzz, OUTPUT);
  pinMode(mot, OUTPUT);
  //v todo lo de adentro del elevador v
  pinMode(s1, OUTPUT);
  pinMode(s2, OUTPUT);
  pinMode(s3, OUTPUT);
  //debajo de esto van todos los FC
  pinMode(fc1, OUTPUT);
  pinMode(fc2, OUTPUT);
  pinMode(fc3, OUTPUT);
  // todo el resto
  pinMode(led, OUTPUT);
}

void loop() {
  int peso = analogRead(A0);
  //v todo lo de adentro del elevador v
  int s1 = digitalRead(13); // conectado bien y funciona bien
  int s2 = digitalRead(12); // conectado bine y funciona bien
  int s3 = digitalRead(11); // conectado bien y funciona bien
  // v finales de carrera v
  int fc1 = digitalRead(10);
  int fc2 = digitalRead(9);
  int fc3 = digitalRead(8);
  // cada piso
  int s5 = digitalRead(13);
  int s6 = digitalRead(12);
  int s7 = digitalRead(11);
  //mantenitmiento
  int mant = digitalRead(4);
  
  // parte de parada por mantenimiento no funca
  if (mant == true){
    digitalWrite(mot, LOW);
  }
  if (peso > 900) {
    digitalWrite(buzz, HIGH);
    digitalWrite(mot, LOW);
  }
  if (peso < 900) {
    digitalWrite(buzz, LOW);
  }
  /*todo lo que es subida va aca
   *para subir de piso 1 a 2
  */
  if (peso < 900 && s2 == true && fc1 == true) {
    digitalWrite(mot, HIGH);
    delay(5000);
    digitalWrite(mot, LOW);
    }
    //para subir de piso 1 a 3
  if(peso < 900 && s3 == true && fc1 == true) {
    digitalWrite(mot, HIGH);
    delay(5000);
    digitalWrite(mot, LOW);
  }
  //para subir de piso 2 a 3
  if(peso < 900 && s3 == true && fc2 == true) {
    digitalWrite(mot, HIGH);
    delay(5000);
    digitalWrite(mot, LOW);
  }
  /* Aca va todo lo que es bajada
   * piso 3 a 2
   */
   if(peso < 900 && s2 == true && fc3 == true) {
    digitalWrite(mot, HIGH);
    delay(5000);
    digitalWrite(mot, LOW);
   }
   // piso 3 a 1
   if(peso < 900 && s1 == true && fc3 == true) {
    digitalWrite(mot, HIGH);
    delay(5000);
    digitalWrite(mot, LOW);
   }
   /*parada de mantenimiento
    * Cuando esta apretado se parpadea un led y cuando se aprieta un boton de mando se hazze un chillido el buzzer
    */
    // parte del led sin llamado
if(mant == true) {
      digitalWrite(led, HIGH);
      delay(500);
      digitalWrite(led, LOW);
      delay(500);
    }
    // parte con el mando
    if(mant == true && s1 == true) {
      digitalWrite (buzz, HIGH);
      delay(100);
      digitalWrite (buzz, LOW);
      delay(100);
      digitalWrite (buzz, HIGH);
      delay(100);
      digitalWrite (buzz, LOW);
      delay(100);
      digitalWrite (buzz, HIGH);
      delay(100);
      digitalWrite (buzz, LOW);
      delay(100);
      digitalWrite(led, HIGH);
      delay(500);
      digitalWrite(led, LOW);
      delay(500);
    }
   }

 

Link to comment
Share on other sites

Link to post
Share on other sites

1 hour ago, James Evens said:

without even looking at your code did you used automata theory?

 

Now let's look at your code:

- spanish comments (fine but they don't help when posting it online)

- peso is a local and global variable!

- since everything is spanish: what are your parameter (simular condition)? which if?

the problem aint in that part but only in the last 2 if statements everything else works perfect

Link to comment
Share on other sites

Link to post
Share on other sites

Also, the only reason why it is in spanish is cause it is for school and they don't understand englsh. I get your point but changing all the integrers and explananations to english is a lot of work for just 1 problem I'd do it but I didn't have the time. None the less, the problem is in this part. (check the quotes). and to your question, yes I did use a lot of that mainly due to me being a noob at it and when I lookdd on the internet there were thing like the switch command that I couldn' t get it to work. 

Quote

if(mant == true) {

      digitalWrite(led, HIGH);

      delay(500);

      digitalWrite(led, LOW);

      delay(500);

    }

    // parte con el mando

    if(mant == true && s1 == true) {

      digitalWrite (buzz, HIGH);

      delay(100);

      digitalWrite (buzz, LOW);

      delay(100);

      digitalWrite (buzz, HIGH);

      delay(100);

      digitalWrite (buzz, LOW);

      delay(100);

      digitalWrite (buzz, HIGH);

      delay(100);

      digitalWrite (buzz, LOW);

      delay(100);

      digitalWrite(led, HIGH);

      delay(500);

      digitalWrite(led, LOW);

      delay(500);

    }

 

Link to comment
Share on other sites

Link to post
Share on other sites

1 wheres the text/description of the problem, the requirements - am i supposed to guess what your code has to do?

2 dunno Spanish, please use English comments

3 give variables names that make more sense , s1,s2, s3 , fc1,fc2,fc3 don't tell me anything. and btw  "s" in general is used for strings - most programmers avoid declaring integer variables starting with s

4 what's the deal with int everywhere?  int uses 2 bytes in arduino, while digitalRead returns only HIGH or LOW which can be easily stored in a single byte... All your other variables  except peso i think can fit in 1 byte

5 you have if peso<900 and if peso > 900 but what happens when/if peso is actually 900?

6 can't be bothered to really go through the code but often people forget to use if then else  and use only if  or don't close the {} where they should be... you have a weird indentation for an if as if you were supposed to not close a } ... go through the logic again.

 

Link to comment
Share on other sites

Link to post
Share on other sites

9 hours ago, James Evens said:

Your problem is that both get triggered?

Use a else if.

if(mant == true && s1 == true) {

[...]

} else if(mant == true && !s1) {

[...]

 

alternatively (the better solution) (not arduino, just a normal c++ terminal app):


#include <iostream>
int main()
{
    bool mant = true;
    bool s1 = true;
    if (mant && s1) {
        std::cout << "true";
    }
    if (mant && !s1) {
        std::cout << "false";
    }
    return 0;
}

 

The problem kinda lies in there, When I press the "mant" button the led goes on and off but if I press s1 the buzzard doesn't sound. I have to do the same thing for s2 and s3. 

 

just fyi, mant is a button for maintenance as long as that is pressed elevator won't go anywhere and a led will go on and off to mark someone's working on the elevator. Now s1, s2 and s3 are the buttons to send the elevator to floor 1,2 and 3. Where just in case one did not see the led when the button gets pressed a buzzard will sound alongst with the led that keeps going on and off. 

Link to comment
Share on other sites

Link to post
Share on other sites

And yes c++ is obv better but it is just a prject we have to make "hypothetically". And I got told that I should try to have the circuit physically working on arduino. 

Link to comment
Share on other sites

Link to post
Share on other sites

if(mant == true && s1 == true) {
      digitalWrite (buzz, HIGH);
      delay(100);
      digitalWrite (buzz, LOW);
      delay(100);
      digitalWrite (buzz, HIGH);
      delay(100);
      digitalWrite (buzz, LOW);
      delay(100);
      digitalWrite (buzz, HIGH);
      delay(100);
      digitalWrite (buzz, LOW);
      delay(100);
      digitalWrite(led, HIGH);
      delay(500);
      digitalWrite(led, LOW);
      delay(500);
    } else if(mant == true) {
      digitalWrite(led, HIGH);
      delay(500);
      digitalWrite(led, LOW);
      delay(500);
    }

just swapping the two around and making it an else if should sort your issue. that way if the first step is met (the buzzer sounded) then the second will be ignored

 

you could also use a for loop to tidy up the buzzing like this:

if(mant == true && s1 == true) {
	for (int i=0; i<3; i++) {
		digitalWrite (buzz, HIGH);
		delay(100);
		digitalWrite (buzz, LOW);
		delay(100);
	}
	digitalWrite (led, HIGH);
	delay(500);
	digitalWrite(led, LOW);
	delay(500);
	}

 

Link to comment
Share on other sites

Link to post
Share on other sites

also to make it work for all of them try

if(mant == true && (s1 == true || s2 == true || s3 == true)) {
Link to comment
Share on other sites

Link to post
Share on other sites

On 10/19/2018 at 5:11 AM, James Evens said:

alternatively (the better solution) (not arduino, just a normal c++ terminal app):

Somewhat off-topic and at the risk of sounding pedantic:

The optimal solution is to not test the same variable twice if you can help it. Which helps any reader with following the logic when things get more complex:

#include <iostream>
int main()
{
	bool mant = true;
	bool s1 = true;

	if (mant)
	{
		if (s1)
		{
			std::cout << "true";
		}
		else
		{
			std::cout << "false";
		}
	} 

	return 0;	
}

 

Link to comment
Share on other sites

Link to post
Share on other sites

On 10/20/2018 at 1:39 AM, James Evens said:

Arduino is c++. Only std::cout and iostream will not work. The Boolean logic is the same.

If you interested in computer science. Read about automates. For your problem a nea/dea is enough (push down automate would make mate easier): https://en.wikipedia.org/wiki/Nondeterministic_finite_automaton

 

TL;DR you define states and transition rules

 

 

2 hours ago, Unimportant said:

Somewhat off-topic and at the risk of sounding pedantic:

The optimal solution is to not test the same variable twice if you can help it. Which helps any reader with following the logic when things get more complex:


#include <iostream>
int main()
{
	bool mant = true;
	bool s1 = true;

	if (mant)
	{
		if (s1)
		{
			std::cout << "true";
		}
		else
		{
			std::cout << "false";
		}
	} 

	return 0;	
}

 

Huge thanks to your helpful replies! I will look into it tomorrow when I am free! :D

Link to comment
Share on other sites

Link to post
Share on other sites

2 hours ago, Unimportant said:

Somewhat off-topic and at the risk of sounding pedantic:

The optimal solution is to not test the same variable twice if you can help it. Which helps any reader with following the logic when things get more complex:


#include <iostream>
int main()
{
	bool mant = true;
	bool s1 = true;

	if (mant)
	{
		if (s1)
		{
			std::cout << "true";
		}
		else
		{
			std::cout << "false";
		}
	} 

	return 0;	
}

 

In terms of what you said could you explain a bit more? I am quite a big noob when it comes to arduino so idrk what you said there lol

Link to comment
Share on other sites

Link to post
Share on other sites

I think he means this:

 

If you write it like this:

 

if a == true && x == something then do stuff ..

if a == true && y == something then do stuff .. 

 

This is not the most efficient way to do things, because at the start of each if the processor is forced to check if a is equal to true, so when the processor reaches the second IF it doesn't remember that it already compared a to true and determined the result in the previous if, the processor will do what you say and check if a is still true .

So you have four comparisons.

 

you can rewrite that as

 

if a == true  then

    if x == something then ...

    if y == something then ....

(end if a == true)

 

Now you processor only has to calculate 3 comparisons so your code is faster and less flash memory on the microcontroller is used and it's easier to follow the flow with your eyes

.

 

Link to comment
Share on other sites

Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×