Motor driver code does not work

Hello, i was here a while ago and got help with my code for a project in working on and today the arduino finally arrived in the mail so i could start testing, and it didn't work as expected. I wrote a another quick program and the motor could spin in 2 directions thanks to the L293D IC.
The motor is supposed to spin a leadscrew that will move a small "wagon". On each side of the track where the wagon will go there will be sensors to see where the wagon, so the arduino can decide which was to spin the motor. The sensors that control that are named "kontrollPin1" and "kontrollPin2". The main switch is named "knapp".

I don't understand why the code won't work. i think there is another flaw too. If kontrollPin1 reads low first, then the wagon will be moved to the other side as long as the sensor on the side the wagon is moving towards is not sensing the wagon. But when i have moved over the other side will say it is missing the wagon and the it will move back to the original position, how can i avoid that?
thanks.

const int motorPin1 = 10;
const int motorPin2 = 9;
const int knappPin = 8;

const int kontrollPin1 = 7;
const int kontrollPin2 = 6;

boolean kontroll1 = HIGH;
boolean kontroll2 = HIGH;
boolean knapp = HIGH;

int val;
int val2;

void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);

  knapp = digitalRead(knappPin);
}


void loop()
{
  kontroll1 = digitalRead(kontrollPin1);
  kontroll2 = digitalRead(kontrollPin2);

  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (val == val2) 
  {
    if (val != knapp) 
    {
      if (val == LOW) 
      {
        if (kontroll1 == LOW) 
        {
          while (digitalRead(kontrollPin1) != LOW) 
          {
            digitalWrite(motorPin1, HIGH);
          }
        }
        else 
        {
          digitalWrite(motorPin1, LOW);
        }
        if (kontroll2 == LOW) 
        {
          while (digitalRead(kontrollPin2) != LOW) 
          {
            digitalWrite(motorPin2, HIGH);
          }
        }
        else 
        {
          digitalWrite(motorPin2 , LOW);
        }
      }
    }
  }         
}

Most of your code depends on the value of knapp that was read in setup(). Why is that? What kind of switch is connected to knappPin?

          while (digitalRead(kontrollPin1) != LOW) 
          {
            digitalWrite(motorPin1, HIGH);
          }

You don't need to repeatedly set the pin HIGH. It stays HIGH until you set it otherwise,

The wagon is going to move to the other side when i press the button "knapp", when i press knapp i want i.e a box to open if it was closed and vice versa, kontrollPin1 and 2 decides if the box is open or closed.. Knapp is normally open, does not allow the flow of current.

i fixed the while statement.
Could i maybe use switch case to make sure the wagon does not move back and forth?

const int motorPin1 = 10;
const int motorPin2 = 9;
const int knappPin = 8;

const int kontrollPin1 = 7;
const int kontrollPin2 = 6;

boolean kontroll1 = HIGH;
boolean kontroll2 = HIGH;
boolean knapp = HIGH;

int val;
int val2;

void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);

  knapp = digitalRead(knappPin);
}


void loop()
{
  kontroll1 = digitalRead(kontrollPin1);
  kontroll2 = digitalRead(kontrollPin2);

  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (val == val2) 
  {
    if (val != knapp) 
    {
      if (val == LOW) 
      {
        if (kontroll1 == LOW) 
        {
          digitalWrite(motorPin1, HIGH);
          while (digitalRead(kontrollPin1) != LOW) 
          {
          }
        }
        else 
        {
          digitalWrite(motorPin1, LOW);
        }
        if (kontroll2 == LOW) 
        {
          digitalWrite(motorPin2, HIGH);
          while (digitalRead(kontrollPin2) != LOW) 
          {
          }
        }
        else 
        {
          digitalWrite(motorPin2 , LOW);
        }
      }
    }
  }         
}

Way too much nesting, hurts the brain, I strongly recommend
reworking the logic to be more natural - use sensibly named predicate
functions to make the body of loop simpler.

  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (val == val2) 
  {
    if (val != knapp) 
    {
      if (val == LOW) 
      {

is inpentrable!

Bit more understandable as:

  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (!val && !val2 && knapp)
  {
val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (!val && !val2 && knapp)
  {

And that will debounce the button correctly?

No idea, its your code, I'm just making it readable so you can see what its doing.

The wagon is going to move to the other side when i press the button "knapp",

No, it isn't. knapp is the VALUE read from a pin.

That does not answer to question of what kind of switch is attached to that pin, or why you only want to act when that switch is set one way WHEN THE ARDUINO STARTS UP.

Sorry for the confusion, knapp is swedish for button. I read the state of knapp when the arduino boots up instead of writing knapp = HIGH, but i just saw i already did declare knapp as high. i should remove that from the code. Knapp is used to make sure the state of the button had changed, and that i was pressed, not released. The val and val2 would be same even if the button was not pressed, that is why i need knapp.

wildiness:
Sorry for the confusion, knapp is swedish for button. I read the state of knapp when the arduino boots up instead of writing knapp = HIGH, but i just saw i already did declare knapp as high. i should remove that from the code. Knapp is used to make sure the state of the button had changed, and that i was pressed, not released. The val and val2 would be same even if the button was not pressed, that is why i need knapp.

But, you don't record the state of the switch, so next time around, you can't tell whether the switch has changed state, or not. You are debouncing the switch, so you know that the switch is pressed or is released, but not whether it just became pressed or released.

If the idea is that each time the switch BECOMES pressed, the wagon moves from end to the other, debouncing is useless, as the bouncing will have died out long before the wagon has moved to the other end of the track.

Look at the state change detection example, instead.

It is a momentary switch, when it is not pressed the state is always high, that is why i thought i could just check if the state was LOW meaning someone pressed it, then when you stop pressing it becomes HIGH again. Have u been talking about a toggle button, if so, sorry for not telling you it was a momentary switch.

I have changed some things in the code.

const int motorPin1 = 10;
const int motorPin2 = 9;
const int knappPin = 8;

const int kontrollPin1 = 7;
const int kontrollPin2 = 6;

int val;
int val2;
int x;

boolean kontroll;
boolean kontroll2;
boolean knapp;


void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);

  knapp = digitalRead(knappPin);
}


void loop()
{
  kontroll = digitalRead(kontrollPin1);
  kontroll2 = digitalRead(kontrollPin2);
  x = 0;

  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (val == val2) 
  {
    if (val != knapp) 
    {
      if (val == LOW) 
      {
        if (kontroll == LOW && x == 0) 
        {
          x = 1;
          digitalWrite(motorPin1, HIGH);
          while (digitalRead(kontrollPin1) != LOW)
          {
          }
        }
        else 
        {
          digitalWrite(motorPin1, LOW);
        }
        if (kontroll2 == LOW && x == 0) 
        {
          x = 1;
          digitalWrite(motorPin2, HIGH);
          while (digitalRead(kontrollPin2) != LOW) 
          {
          }
        }
        else 
        {
          digitalWrite(motorPin2 , LOW);
        }
      }
    }
  }         
}

It is a momentary switch, when it is not pressed the state is always high, that is why i thought i could just check if the state was LOW meaning someone pressed it, then when you stop pressing it becomes HIGH again.

Then, there is no point in reading the state in setup().

  kontroll = digitalRead(kontrollPin1);
  kontroll2 = digitalRead(kontrollPin2);

The state of these pins is irrelevant at this time.

  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (val == val2)

You are STILL fixated on debouncing, which is NOT needed, since the switch will have stopped bouncing long before the wagon has moved, if there is a need to move the wagon.

Since you have an unconditional delay, the first reading is useless.

    if (val != knapp)

Since knapp is, effectively, a constant, just use the constant here.

      if (val == LOW) 
      {

This test is rather stupid, since you wouldn't have gotten here unless val was LOW.

        if (kontroll == LOW && x == 0)

Right before this is where you wanted to read the value of kontrolPin1.

I have changed the code and removed some things, but i left the debouncing in. does not harm the code, right?

const int motorPin1 = 10;
const int motorPin2 = 9;
const int knappPin = 8;

const int kontrollPin1 = 7;
const int kontrollPin2 = 6;

int val;
int val2;
int x;

boolean kontroll;
boolean kontroll2;
boolean knapp;


void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);

  knapp = digitalRead(knappPin);
}


void loop()
{

  x = 0;

  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (val == val2) 
  {
    if (val != knapp) 
    {
      kontroll = digitalRead(kontrollPin1);
      kontroll2 = digitalRead(kontrollPin2);
      if (kontroll == LOW && x == 0) 
      {
        x = 1;
        digitalWrite(motorPin1, HIGH);
        while (digitalRead(kontrollPin1) != LOW)
        {
        }
      }
      else 
      {
        digitalWrite(motorPin1, LOW);
      }
      if (kontroll2 == LOW && x == 0) 
      {
        x = 1;
        digitalWrite(motorPin2, HIGH);
        while (digitalRead(kontrollPin2) != LOW) 
        {
        }
      }
      else 
      {
        digitalWrite(motorPin2 , LOW);
      }

    }
  }         
}

I have changed the code and removed some things, but i left the debouncing in. does not harm the code, right?

So, this works now?

Yes, almost. The motor started spinning when i pressed the button "knapp". but it did not stop when it was supposed too. i changed the kontrollpin's to simulate the wagon reaching the other end but the motor didnt stop spinning. It did however spin different directions depending on which kontrollpin was LOW and didnt do anything when neither was LOW.

here is the new code

const int motorPin1 = 10;
const int motorPin2 = 9;
const int knappPin = 8;

const int kontrollPin1 = 7;
const int kontrollPin2 = 6;

int val;
int val2;
int x;

boolean kontroll;
boolean kontroll2;
boolean knapp;


void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);

  knapp = digitalRead(knappPin);
}


void loop()
{

  x = 0;

  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (val == val2) 
  {
    if (val != knapp) 
    {
      kontroll = digitalRead(kontrollPin1);
      kontroll2 = digitalRead(kontrollPin2);
      if (kontroll == LOW && x == 0) 
      {
        x = 1;
        digitalWrite(motorPin1, HIGH);
        while (digitalRead(kontrollPin1) != LOW)
        {
        }
        digitalWrite(motorPin1, LOW);
      }
      if (kontroll2 == LOW && x == 0) 
      {
        x = 1;
        digitalWrite(motorPin2, HIGH);
        while (digitalRead(kontrollPin2) != LOW) 
        {
        }
        digitalWrite(motorPin2 , LOW);
      }
    }
  }         
}

And i broke everything. i changed the code again and now nothing happens at all.

const int motorPin1 = 10;
const int motorPin2 = 9;
const int knappPin = 8;

const int kontrollPin1 = 7;
const int kontrollPin2 = 6;

int val;
int val2;
int x;

boolean kontroll;
boolean kontroll2;
boolean knapp;


void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);

  knapp = digitalRead(knappPin);
}


void loop()
{

  x = 0;

  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (val == val2) 
  {
    if (val != knapp) 
    {
      kontroll = digitalRead(kontrollPin1);
      kontroll2 = digitalRead(kontrollPin2);
      if (kontroll == LOW && x == 0) 
      {
        x = 1;
        digitalWrite(motorPin1, HIGH);
        while (digitalRead(kontrollPin1) != LOW)
        {
        }        
      }
      digitalWrite(motorPin1, LOW);
      if (kontroll2 == LOW && x == 0) 
      {
        x = 1;
        digitalWrite(motorPin2, HIGH);
        while (digitalRead(kontrollPin2) != LOW) 
        {
        }        
      }
      digitalWrite(motorPin2 , LOW);
    }
  }         
}
  knapp = digitalRead(knappPin);

Stop doing this. It's useless.

When you get rid of the stupid debouncing code, I'll look in again.

Okay, it is commented out. I could not have else after the If because the motorpin was never set LOW again, so i tried moving it inside and now nothing happens when i press the main button "knapp" and one of the kontrollpin's are LOW.

const int motorPin1 = 10;
const int motorPin2 = 9;
const int knappPin = 8;

const int kontrollPin1 = 7;
const int kontrollPin2 = 6;

int val;
int val2;
int x;

boolean kontroll;
boolean kontroll2;
boolean knapp = HIGH;


void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);

  //knapp = digitalRead(knappPin);
}


void loop()
{

  x = 0;

  val = digitalRead(knappPin);
  //delay(10);
  //val2 = digitalRead(knappPin);

  //if (val == val2) 
  //{
  if (val != knapp) 
  {
    kontroll = digitalRead(kontrollPin1);
    kontroll2 = digitalRead(kontrollPin2);
    if (kontroll == LOW && x == 0) 
    {
      x = 1;
      digitalWrite(motorPin1, HIGH);
      while (digitalRead(kontrollPin1) != LOW)
      {
      }
      digitalWrite(motorPin1, LOW);
    }
    if (kontroll2 == LOW && x == 0) 
    {
      x = 1;
      digitalWrite(motorPin2, HIGH);
      while (digitalRead(kontrollPin2) != LOW) 
      {
      }
      digitalWrite(motorPin2 , LOW);
    }
  }
  //}         
}

Why don't you go have a look at the state change detection example? Create a sketch that does nothing except print that the switches became pressed or became released. Verify that all three switches are working properly before you try to make the motor do anything. Get rid of x. It is NOT needed. DO NOT COMMENT STUFF OUT. The delete key is there for a reason.

What @PaulS said ... plus ...

If you organize your program into separate functions it will be much easier to follow. For example something like this pseudo code

void setup() {
   // whatever initialization is needed
}

void loop() {
    readButtons();
    moveMotor();
}

void readButtons() {
  // read the state of each button and save it to a global variable
}

void moveMotor() {
   // move or stop the motor depending on the button states saved in the global variables
}

...R

I removed some stuff and changed the code so it writes to serial what buttons are pressed and they all work. But the code does not pause at the while statement like i want it to, it should only continue with the rest of the code when the while statement is no longer true, but it does not, it just loops again directly.
I also looked and ran the StateChangeDetection code and it really needs a debounce.

Robin2: I've seen functions like that but since i am new to C i have chosen to not get to complicated too early.

Here is the code that successfully writes to serial but jumps the while statement, maybe that is where the fault lies?

const int motorPin1 = 10;
const int motorPin2 = 9;
const int knappPin = 8;

const int kontrollPin1 = 7;
const int kontrollPin2 = 6;

int val;
int val2;
int x;
int y;

boolean kontroll;
boolean kontroll2;
boolean knapp = HIGH;


void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);

  Serial.begin(9600);
}


void loop()
{
  x = 0;

  val = digitalRead(knappPin);
  if (val != knapp) 
  {
    Serial.print("val: ");
    Serial.println(val);
    kontroll = digitalRead(kontrollPin1);
    kontroll2 = digitalRead(kontrollPin2);
    Serial.print("k1: ");
    Serial.println(kontroll);
    Serial.print("k2: ");
    Serial.println(kontroll2);
    if (kontroll == LOW && x == 0) 
    {
      x = 1;
      digitalWrite(motorPin1, HIGH);
      while (digitalRead(kontrollPin1) != LOW)
      {
      }
      digitalWrite(motorPin1, LOW);
    }
    if (kontroll2 == LOW && x == 0) 
    {
      x = 1;
      digitalWrite(motorPin2, HIGH);
      while (digitalRead(kontrollPin2) != LOW) 
      {
      }
      digitalWrite(motorPin2 , LOW);
    }
  }        
}