C++ command based issues

C++ command based issue

I’ve got a simple solenoid powered claw example
All of the command creation and binding is done in Manipulator.

I bind a toggle command to the Dashboard and to a button on the controller.

The dashboard command’s run button isn’t showing and when I press start I get this error

Illegal use of Command: Commands that have been composed may not be added to another composition or scheduled individually!

I don’t think I’m individually scheduling commands apart from the composition

https://github.com/narmstro2020/Tutorials_CPP_2023-2024/tree/Claw_Tutorial_01

That link 404’s for me, is the repo private or is that branch not pushed?

Sorry. Didn’t set to public. Should be good now.

Sorry again. Now it should be good

Your repository name suggests that you’re following a tutorial. Could you also provide a link to that tutorial? If you’re not following a tutorial, I don’t see any commands in your code. What are you trying to accomplish?

I suspect your commands are going out of scope before they’re bound. Make sure you return rvalues from your factories.

I’m not super familiar with C++ wpilib, but according to the WPILIbC examples (Hatchbot inlined), the button binding seems to accept a CmdPtr, rather than the Command.

So, have you tried

frc::SmartDashboard::PutData(testToggleCommand);

in place of:

frc::SmartDashboard::PutData(testToggleCommand.get());

Not sure if that matters or not. Just an interesting difference. I don’t quite understand why you’re instantiating a long list of commands in your toggle commands structure of Manipulator so some explanation of what you’re trying to do may help as well.

CreateClawToggleStateCommand() is wrong. You move openLeftCommand into openClaw, and then return an empty openLeftCommand CommandPtr. Thats why it doesn’t show up on the dashboard.

As for the composition error, its the same issue, but also maybe shows a bug on our end. You’re taking openLeftCommand out of CreateClawToggleStateCommand(), which has been composed into openClaw(), and binding it to a trigger.

Beta 4 has some fixes to make these bugs clearer and throw errors earlier.

1 Like

Sorry about that. I was doing some debugging and just wanted to test that part. I didn’t reset it all the way. It’s back the way it should be on the repo, but I’m still getting the same behavior

I made this already in Java for my kiddos. I wanted to also make a c++ version to see if I can do it.

I have. I get this

no instance of overloaded function “frc::SmartDashboard::PutData” matches the argument list

I put another layer between Robot Container and the subsystems just so that Robot Container doesn’t get too messy. I call it Manipulator. The commands are there

Got it. Updated github with fix.

So am I right in thinking that, unlike Java, C++ needs to have commands stored somewhere in the header file if I’m to use them the way I’m using them in this little project.

I can just make the command in a method in Java and bind it to a trigger no problem.

You do not need to store temporaries as fields. If you’ve used std::move properly (or else just composed with rvalues directly from constructors), nothing will go out of scope.

The safest thing to do is compose inline. If you find it getting verbose, add an intermediate factory.

Could you provide an example of using std::move properly. New to C++

It looks like you mostly have the idea of it; you use it whenever you are transferring ownership to the other side of the call contract. When I compose commands, the component command’s ownership should transfer to the parent command, so we have to std::move it when it is passed.

Okay. what am I missing then. Because this only works now when I have the commands (both testing and button) stored as fields.

When you bind the command to a trigger, you need to give ownership to the trigger for the same reason. As your code is written, the command goes out of scope immediately after binding.

so like this

frc2::Trigger trigger = commandXBoxController->Button(std::move(toggleButton));

and likewise for the testing command

frc::SmartDashboard::PutData(std::move(clawToggleCommandTest.get()));

I implore you to just inline constructors/factory calls wherever feasible instead of doing all this stuff out-of-line and then using std::move. It’s something of a code smell.

You do not need to use std::move when getting a Trigger from the CommandXboxController; the returned Trigger is already owned by the controller.

You do not need to call .get on the CommandPtr instance, either; you can/should move the entire command.