C++ Toggle Button

[thelegend]

I am trying to make a toggle for switching between Arcade Drive and Tank drive. I wrote and tested this code but the only result I get is a switch between Tank to Arcade, not the other way back. I have such a complicated system because I believe that if I simply set a variable to be True or False, it won't compensate for if the driver holds on to the button. That's why I have a clause in there that checks if the button has been released.

If you find any issues, please reply!

void OperatorControl(void)
	{
		bool boolval = false;
		bool toggle = true;
		bool toggle2 = false;
		
		feeder->Set(false);
		firingMech->Set(true);
		
		while (IsOperatorControl())
		{
			if(boolval){
			//Arcade Drive
				//Left Side
				leftMiddleMotor -> SetSpeed(primaryController -> GetRawAxis(5) - primaryController -> GetRawAxis(4));
				leftBottomMotor -> SetSpeed(primaryController -> GetRawAxis(5) - primaryController -> GetRawAxis(4));
				//Right Side
				rightTopMotor -> SetSpeed(-(primaryController -> GetRawAxis(5) + primaryController -> GetRawAxis(4)));
				rightMiddleMotor -> SetSpeed(-(primaryController -> GetRawAxis(5) + primaryController -> GetRawAxis(4)));
				rightBottomMotor -> SetSpeed(-(primaryController -> GetRawAxis(5) + primaryController -> GetRawAxis(4)));
				if(!(secondaryController -> GetRawButton(6))){
					toggle2 = true;
					printf("Status is: \n");
				} 
				if(secondaryController -> GetRawButton(6) && toggle2 == true){
					boolval = false;
					toggle2 = false;
					toggle = true;
				}
			}
			//Arcade - Tank Toggle Switch
				if(secondaryController -> GetRawButton(6)){
					boolval = true;
					toggle = false;
				}else if(toggle){
					// Left Motor Drivetrain
					if (primaryController -> GetRawAxis(2)){
							leftMiddleMotor -> SetSpeed(primaryController -> GetRawAxis(2));
							leftBottomMotor -> SetSpeed(primaryController -> GetRawAxis(2));
						}else{
							leftMiddleMotor -> SetSpeed(0.0);
							leftBottomMotor -> SetSpeed(0.0);
						}
					//Right Motor Drivetrain
					if (primaryController -> GetRawAxis(5)){
							rightTopMotor -> SetSpeed(-(primaryController -> GetRawAxis(5)));
							rightMiddleMotor -> SetSpeed(-(primaryController -> GetRawAxis(5)));
							rightBottomMotor -> SetSpeed(-(primaryController -> GetRawAxis(5)));
						}else{
							rightTopMotor -> SetSpeed(0.0);
							rightMiddleMotor -> SetSpeed(0.0);
							rightBottomMotor -> SetSpeed(0.0);
						}					//Tank
				}
			// Compression
			if (secondaryController -> GetRawButton(8)){
					compressor -> Set(Relay::kForward);
				}else{
					compressor -> Set(Relay::kOff);
				}

			//Lifter
			if (secondaryController -> GetRawAxis(2)){
					leftLifterMotor -> SetSpeed(secondaryController -> GetRawAxis(2));
					rightLifterMotor -> SetSpeed(-(secondaryController -> GetRawAxis(2)));
				}else{
					leftLifterMotor -> SetSpeed(0.0);
					rightLifterMotor -> SetSpeed(0.0);
				}
			//Conveyer
				//Conveyer go up
			if (secondaryController -> GetRawButton(3)){
					conveyer -> SetSpeed(1.0);
				}else{
					conveyer -> SetSpeed(0.0);
				}
				//Conveyer go down
			if (secondaryController -> GetRawButton(2)){
					conveyer -> SetSpeed(-1.0);
				}else{
					conveyer -> SetSpeed(0.0);
				}
			//Shooter Motor
			if(secondaryController -> GetRawButton(5)){
					shooterInner -> SetSpeed(-(secondaryController -> GetRawAxis(3)));
					shooterOuter -> SetSpeed(-(secondaryController -> GetRawAxis(3)));
				}else{
					shooterInner -> SetSpeed(0.0);
					shooterOuter -> SetSpeed(0.0);
				}
			//Shooter FIRE!!!!
			if(secondaryController -> GetRawButton(1)){
					firingMech -> Set(true);
				}else{
					firingMech -> Set(false);	
				}
			//Arouser
				//Arouser Up
			if(secondaryController -> GetRawButton(11)){
					feeder -> Set(true);
				}
				//Arouser Down
			if(secondaryController -> GetRawButton(10)){
					feeder -> Set(false);
				}
			}		
			Wait(0.05);
			
		
	}

[Domenic Rodriguez]

One way to increase the readability of your code would be to use an enum to keep track of the drive mode. This also lets you simplify the logic for tracking the button press state:

void OperatorControl(void)
{
    typedef enum _DriveMode { kArcadeDrive, kTankDrive } DriveMode;

    DriveMode mode = kArcadeDrive;
    bool current, prev;
    prev = current = false;

    while (IsOperatorControl())
    {
        // Check the mode toggle button
        current = secondaryControler->GetRawButton(6);
        if (mode == kArcadeDrive) {
            // Arcade Drive code here...

            // Check for state chagne
            if (current && !prev)
                mode = kTankDrive;
        } else if (mode == kTankDrive) {
            // Tank Drive code here...

            // Check for state change
            if (current && !prev)
                mode = kArcadeDrive;
        }

        // Set old button value for next iteration
        prev = current;
    }

    // Other code goes here...
}

[jmullins16]

Very nice answer. And to bring out a couple more subtle points displayed in the answer that will help understandability.

- Don't be afraid to use a bit more whitespace. Parentheses, brackets, and keywords all deserve attention and the whitespace gives the user visual focus. The answer example has a good balance of spacing.

- The code for managing the toggling is now grouped very tight and easily understood without paging around the drive code packed into it. This can be maintained by either putting the drive code into their own sub-functions, or simply making a new section immediately after the toggling code which re-checks the arcade/tank enum solely for the purpose of doing just the drive functions. There is no penalty for re-checking a value such as this.

[mikets]

This is one of the programming topics I teach my students: how to make something happen when a button is pressed or released. The first challenge is to detect button presses and releases as an edge event. The second challenge is to toggle a mode when a button is pressed. For the first challenge, detecting an edge event means to detect the transition between two states which means you need to remember the previous state and compare it with the current state. This could be done with two Boolean variables.

bool prevState = false;
bool currState = false;

while (IsOperatorControl())
{
    //
    // Check for button 6 press.
    //
    currState = secondaryController->GetRawButton(6);
    if (currState != prevState)
    {
        if (currState)
        {
            //
            // Button has been pressed.
            //
        }
        else
        {
            //
            // Button has been released.
            //
        }
        prevState = currState;
    }

    wait(0.02);
}

For the second challenge, you simply toggle a Boolean variable like this:

bool driveTankMode = false;

driveTankMode = !driveTankMode;

So put them all together:

bool prevState = false;
bool currState = false;
bool driveTankMode = false;

while (IsOperatorControl())
{
    //
    // Check for button 6 press.
    //
    currState = secondaryController->GetRawButton(6);
    if (currState != prevState)
    {
        if (currState)
        {
            //
            // Button has been pressed.
            //
            driveTankMode = !driveTankMode;
        }
        else
        {
            //
            // Button has been released.
            //
        }
        prevState = currState;
    }

    if (driveTankMode)
    {
        //
        // Do tank drive.
        //
   }
    else
    {
        //
        // Do arcade drive.
        //
   }

    wait(0.02);
}

You can use the edge event detection code to not only toggle a mode, you can do different things when a button is pressed and released. For example, some teams like to "slow down the robot when driving with a button pressed and held down". So you could have the "pressed event" to set the joystick divisor to 2.0 and the "released event" to set the divisor back to 1.0.
Our team even went further to define a joystick button class where the code will be called periodically to check for all button changes. When any button has changed state, it will do an event callback with the parameters specifying which joystick, which button and whether it was a pressed or released event. This greatly simplify our code. For example, our joystick class is TrcJoystick.

class MyRobot
    : public SimpleRobot
    , public ButtonNotify
{
private:
    TrcJoystick m_leftDriveStick;
    TrcJoystick m_rightDriveStick;

public:
    MyRobot(void)
        : m_leftDriveStick(1, this)    //joystick 1, call this class for button events
        , m_rightDriveStick(2, this)   //joystick 2, call this class for button events
    {
    }

    void NotifyButton(
        UINT32 joystickNum,
        UINT16 bitButton,
        bool   fPressed
        )
    {
        if (joystickNum == 1)
        {
            switch (bitButton)
            {
                case Logitech_Trigger:
                    break;

                case Logitech_Btn2:
                    break;
            }
        }
        else if (joystickNum == 2)
        {
            switch (bitButton)
            {
                 case Logitech_Trigger:
                    break;

                case Logitech_Btn2:
                    break;
            }
        }
        else if (joystickNum == 3)
        {
            switch (bitButton)
            {
                 case Logitech_Trigger:
                    break;

                case Logitech_Btn2:
                    break;
            }
        }
    }
};

[SoftwareBug2.0]

Consider this:

class Posedge_toggle{
	bool value,last;

	public:
	Posedge_toggle():value(0),last(1){}

	void update(bool sample){
		if(sample&&!last) value=!value;
		last=sample;
	}

	bool get()const{ return value; }
};
//...

Posedge_toggle t;
while (IsOperatorControl())
{
    t.update(secondaryController->GetRawButton(6));
    if(t.get()){
        //do tank drive
    }else{
       //do arcade drive
    }
    wait(0.02);
}

It decouples the logic of how to make things toggle from the logic to read joysticks, etc.

[mikets]

Sure, how about this? It is basically the same as your Posedge_toggle class except that it allows the update method to also return the switch state so you don't have to make the get call.

class ToggleSwitch
{
private:
    bool switchValue;
    bool prevInput;

public:
    ToggleSwitch(void)
        : switchValue(false)
        , prevInput(false)
    {
    }

    bool GetSwitchValue(void)
    {
        return switchValue;
    }

    bool UpdateSwitchValue(bool currInput)
    {
        if (currInput != prevInput)
        {
            if (currInput)
            {
                switchValue = !switchValue;
            }

            prevInput = currInput;
        }

        return switchValue;
    }
};

...
...
ToggleSwitch tankMode;

while (IsOperatorControl())
{
    //
    // Check for button 6 press.
    //
    if (tankMode.UpdateSwitchValue(secondaryController->GetRawButton(6)))
    {
        //
        // Do tank drive.
        //
    }
    else
    {
        //
        // Do arcade drive.
        //
    }
    wait(0.02);
}

[SoftwareBug2.0]

Originally Posted by mikets

Sure, how about this? It is basically the same as your Posedge_toggle class except that it allows the update method to also return the switch state so you don't have to make the get call.

That would work too.

It seems like you've added a lot of unnecessary stuff though. You're made the class's code go from 13 lines to 33 while you could have made it do that by just adding one line:

class Posedge_toggle{
	bool value,last;

	public:
	Posedge_toggle():value(0),last(1){}

	bool update(bool sample){
		if(sample&&!last) value=!value;
		last=sample;
		return value; //this is the only new line
	}

	bool get()const{ return value; }
};

As for whether the code ought to work this way, I guess it's a matter of taste, but I prefer command-query seperation when possible.

[mikets]

Interesting...
I ran a tool that counts line of code on the ToggleSwitch class code and it gave me:

                         Commented Comment Comment         LOC/semi
           Lines    LOCS      LOCS   Lines   Ratio   Semis    Ratio File Name
              32      16         0       0    0.00       7     2.29 togglesw.cpp

Totals        32      16         0       0    0.00       7     2.29

I ran the same tool on the Posedge_toggle class code and it gave me:

                         Commented Comment Comment         LOC/semi
           Lines    LOCS      LOCS   Lines   Ratio   Semis    Ratio File Name
              14      10         1       0    0.05       6     1.67 posedge.cpp
Totals        14      10         1       0    0.05       6     1.67

It is true that my total number of lines in the file is 32 but that includes blank lines and potential comments which makes the code readable. Therefore, it is more meaningful to count real lines of code. The actual number of lines of real code is only 16. Teaching the students to write readable code is one of my priorities and breaking the lines up is one of the techniques. So I just want to make sure students reading it understand it better.

[SoftwareBug2.0]

Originally Posted by mikets

I ran a tool that counts line of code on the ToggleSwitch class code ...

What tool are you using? I might be interested in trying that out.

I think there is value in having a count of physical lines too though. For example, it controls how much you can see in an editor.

[mikets]

Originally Posted by SoftwareBug2.0

What t ool are you using? I might be interested in trying that out.

That's a proprietary tool from where I worked. But I am sure there are lots of similar open sourced or free tools you can find on the Internet. In fact, I just googled "source code line counter" and got a whole bunch of hits.

[SoftwareBug2.0]

Originally Posted by mikets

That's a proprietary tool from where I worked. But I am sure there are lots of similar open sourced or free tools you can find on the Internet. In fact, I just googled "source code line counter" and got a whole bunch of hits.

Eh, I was mostly interested in trying that exact tool because I can't figure out why it said 32 instead of 33 lines.

[mikets]

Originally Posted by SoftwareBug2.0

Eh, I was mostly interested in trying that exact tool because I can't figure out why it said 32 instead of 33 lines.

That could be due to an extra blank line removed when I copied and pasted the code into notepad when doing the counting. It all depends on the editor if they add/remove blank line at the end of the file (i.e. an extra 0x0d/0x0a).

[SoftwareBug2.0]

Originally Posted by mikets

That could be due to an extra blank line removed when I copied and pasted the code into notepad when doing the counting. It all depends on the editor if they add/remove blank line at the end of the file (i.e. an extra 0x0d/0x0a).

But that would possibly get you one more line than expected rather than one less.

[mikets]

Originally Posted by SoftwareBug2.0

But that would possibly get you one more line than expected rather than one less.

Like I said, I don't really care about the physical number of lines whether it's more or less, I only care about the readability of the code.

[SoftwareBug2.0]

Originally Posted by mikets

Like I said, I don't really care about the physical number of lines whether it's more or less, I only care about the readability of the code.

It may render differently in your browser, but reading the longer version requires me to scroll. When things get longer it doesn't always make them easier to read.