Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[commands] Clarified error messages for parallel composition commands #7161

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

package edu.wpi.first.wpilibj2.command;

import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;

Expand Down Expand Up @@ -51,9 +51,26 @@ public final void addCommands(Command... commands) {
CommandScheduler.getInstance().registerComposedCommands(commands);

for (Command command : commands) {
if (!Collections.disjoint(command.getRequirements(), getRequirements())) {
var sharedRequirements = new HashSet<>(this.getRequirements());
sharedRequirements.retainAll(command.getRequirements());
if (!sharedRequirements.isEmpty()) {
StringBuilder sharedRequirementsStr = new StringBuilder();
boolean first = true;
for (Subsystem requirement: sharedRequirements) {
if (first) {
first = false;
} else {
sharedRequirementsStr.append(", ");
}
sharedRequirementsStr.append(requirement.getName());
}
throw new IllegalArgumentException(
"Multiple commands in a parallel composition cannot require the same subsystems");
String.format(
"Command %s could not be added to this ParallelCommandGroup"
+ " because the subsystems [%s] are already required in this command."
+ " Multiple commands in a parallel composition cannot require"
+ " the same subsystems.",
command.getName(), sharedRequirementsStr));
}
m_commands.put(command, false);
addRequirements(command.getRequirements());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
package edu.wpi.first.wpilibj2.command;

import edu.wpi.first.util.sendable.SendableBuilder;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;

Expand Down Expand Up @@ -81,9 +81,26 @@ public final void addCommands(Command... commands) {
CommandScheduler.getInstance().registerComposedCommands(commands);

for (Command command : commands) {
if (!Collections.disjoint(command.getRequirements(), getRequirements())) {
var sharedRequirements = new HashSet<>(this.getRequirements());
sharedRequirements.retainAll(command.getRequirements());
if (!sharedRequirements.isEmpty()) {
StringBuilder sharedRequirementsStr = new StringBuilder();
boolean first = true;
for (Subsystem requirement: sharedRequirements) {
if (first) {
first = false;
} else {
sharedRequirementsStr.append(", ");
}
sharedRequirementsStr.append(requirement.getName());
}
throw new IllegalArgumentException(
"Multiple commands in a parallel group cannot require the same subsystems");
String.format(
"Command %s could not be added to this ParallelCommandGroup"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest avoiding the word ParallelCommandGroup as that class is only one of the ones relevant. "Parallel composition" would be more accurate.

+ " because the subsystems [%s] are already required in this command."
+ " Multiple commands in a parallel composition cannot require"
+ " the same subsystems.",
command.getName(), sharedRequirementsStr));
}
m_commands.put(command, false);
addRequirements(command.getRequirements());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

package edu.wpi.first.wpilibj2.command;

import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Set;

Expand Down Expand Up @@ -52,9 +52,26 @@ public final void addCommands(Command... commands) {
CommandScheduler.getInstance().registerComposedCommands(commands);

for (Command command : commands) {
if (!Collections.disjoint(command.getRequirements(), getRequirements())) {
var sharedRequirements = new HashSet<>(this.getRequirements());
sharedRequirements.retainAll(command.getRequirements());
if (!sharedRequirements.isEmpty()) {
StringBuilder sharedRequirementsStr = new StringBuilder();
boolean first = true;
for (Subsystem requirement: sharedRequirements) {
if (first) {
first = false;
} else {
sharedRequirementsStr.append(", ");
}
sharedRequirementsStr.append(requirement.getName());
}
throw new IllegalArgumentException(
"Multiple commands in a parallel composition cannot require the same subsystems");
String.format(
"Command %s could not be added to this ParallelCommandGroup"
+ " because the subsystems [%s] are already required in this command."
+ " Multiple commands in a parallel composition cannot require"
+ " the same subsystems.",
command.getName(), sharedRequirementsStr));
}
m_commands.add(command);
addRequirements(command.getRequirements());
Expand Down
14 changes: 9 additions & 5 deletions wpilibNewCommands/src/main/native/cpp/frc2/command/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <string>
#include <utility>
#include <vector>
Daniel1464 marked this conversation as resolved.
Show resolved Hide resolved

#include <wpi/StackTrace.h>
#include <wpi/sendable/SendableBuilder.h>
Expand Down Expand Up @@ -218,12 +219,15 @@ void Command::InitSendable(wpi::SendableBuilder& builder) {
}

namespace frc2 {
bool RequirementsDisjoint(Command* first, Command* second) {
bool disjoint = true;
auto&& requirements = second->GetRequirements();

std::vector<Subsystem*> GetSharedRequirements(Command* first, Command* second) {
std::vector<Subsystem*> shared;
auto&& requirementsOfSecond = second->GetRequirements();
for (auto&& requirement : first->GetRequirements()) {
disjoint &= requirements.find(requirement) == requirements.end();
if (requirementsOfSecond.find(requirement) != requirementsOfSecond.end()) {
shared.push_back(requirement);
}
}
return disjoint;
return shared;
}
} // namespace frc2
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "frc2/command/ParallelCommandGroup.h"

#include <string>
Daniel1464 marked this conversation as resolved.
Show resolved Hide resolved
#include <utility>
#include <vector>

Expand Down Expand Up @@ -75,7 +76,8 @@ void ParallelCommandGroup::AddCommands(
}

for (auto&& command : commands) {
if (RequirementsDisjoint(this, command.get())) {
auto sharedRequirements = GetSharedRequirements(this, command.get());
if (sharedRequirements.empty()) {
command->SetComposed(true);
AddRequirements(command->GetRequirements());
m_runWhenDisabled &= command->RunsWhenDisabled();
Expand All @@ -85,9 +87,23 @@ void ParallelCommandGroup::AddCommands(
}
m_commands.emplace_back(std::move(command), false);
} else {
throw FRC_MakeError(frc::err::CommandIllegalUse,
"Multiple commands in a parallel group cannot "
"require the same subsystems");
std::string formattedRequirements = "";
bool first = true;
for (auto&& requirement : sharedRequirements) {
if (first) {
first = false;
} else {
formattedRequirements += ", ";
}
formattedRequirements += requirement->GetName();
}
throw FRC_MakeError(
frc::err::CommandIllegalUse,
"Command {} could not be added to this ParallelCommandGroup"
" because the subsystems [{}] are already required in this command."
" Multiple commands in a parallel composition cannot require the "
"same subsystems.",
command->GetName(), formattedRequirements);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "frc2/command/ParallelDeadlineGroup.h"

#include <memory>
#include <string>
Daniel1464 marked this conversation as resolved.
Show resolved Hide resolved
#include <utility>
#include <vector>

Expand Down Expand Up @@ -75,7 +76,8 @@ void ParallelDeadlineGroup::AddCommands(
}

for (auto&& command : commands) {
if (RequirementsDisjoint(this, command.get())) {
auto sharedRequirements = GetSharedRequirements(this, command.get());
if (sharedRequirements.empty()) {
command->SetComposed(true);
AddRequirements(command->GetRequirements());
m_runWhenDisabled &= command->RunsWhenDisabled();
Expand All @@ -85,9 +87,23 @@ void ParallelDeadlineGroup::AddCommands(
}
m_commands.emplace_back(std::move(command), false);
} else {
throw FRC_MakeError(frc::err::CommandIllegalUse,
"Multiple commands in a parallel group cannot "
"require the same subsystems");
std::string formattedRequirements = "";
bool first = true;
for (auto&& requirement : sharedRequirements) {
if (first) {
first = false;
} else {
formattedRequirements += ", ";
}
formattedRequirements += requirement->GetName();
}
throw FRC_MakeError(
frc::err::CommandIllegalUse,
"Command {} could not be added to this ParallelCommandGroup"
" because the subsystems [{}] are already required in this command."
" Multiple commands in a parallel composition cannot require the "
"same subsystems.",
command->GetName(), formattedRequirements);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "frc2/command/ParallelRaceGroup.h"

#include <string>
Daniel1464 marked this conversation as resolved.
Show resolved Hide resolved
#include <utility>
#include <vector>

Expand Down Expand Up @@ -62,7 +63,8 @@ void ParallelRaceGroup::AddCommands(
}

for (auto&& command : commands) {
if (RequirementsDisjoint(this, command.get())) {
auto sharedRequirements = GetSharedRequirements(this, command.get());
if (sharedRequirements.empty()) {
command->SetComposed(true);
AddRequirements(command->GetRequirements());
m_runWhenDisabled &= command->RunsWhenDisabled();
Expand All @@ -72,9 +74,23 @@ void ParallelRaceGroup::AddCommands(
}
m_commands.emplace_back(std::move(command));
} else {
throw FRC_MakeError(frc::err::CommandIllegalUse,
"Multiple commands in a parallel group cannot "
"require the same subsystems");
std::string formattedRequirements = "";
bool first = true;
for (auto&& requirement : sharedRequirements) {
if (first) {
first = false;
} else {
formattedRequirements += ", ";
}
formattedRequirements += requirement->GetName();
}
throw FRC_MakeError(
frc::err::CommandIllegalUse,
"Command {} could not be added to this ParallelCommandGroup"
" because the subsystems [{}] are already required in this command."
" Multiple commands in a parallel composition cannot require the "
"same subsystems.",
command->GetName(), formattedRequirements);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>
#include <optional>
#include <string>
#include <vector>
Daniel1464 marked this conversation as resolved.
Show resolved Hide resolved

#include <units/time.h>
#include <wpi/Demangle.h>
Expand Down Expand Up @@ -492,11 +493,11 @@ class Command : public wpi::Sendable, public wpi::SendableHelper<Command> {
};

/**
* Checks if two commands have disjoint requirement sets.
* Gets the shared requirements between two distinct commands.
*
* @param first The first command to check.
* @param second The second command to check.
* @return False if first and second share a requirement.
* @return A vector of shared subsystem requirements.
*/
bool RequirementsDisjoint(Command* first, Command* second);
std::vector<Subsystem*> GetSharedRequirements(Command* first, Command* second);
} // namespace frc2
Loading