From 35862254f9d38e6116102e4fa3f9c8170f20236b Mon Sep 17 00:00:00 2001 From: Piotr Spieker Date: Mon, 18 Nov 2024 15:26:31 +0100 Subject: [PATCH] Handle remaining todo code comments #patch --- include/arbitration_graphs/arbitrator.hpp | 13 ++++++------- test/cost_arbitrator.cpp | 8 ++++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/include/arbitration_graphs/arbitrator.hpp b/include/arbitration_graphs/arbitrator.hpp index 5731b9ed..de6d80d6 100644 --- a/include/arbitration_graphs/arbitrator.hpp +++ b/include/arbitration_graphs/arbitrator.hpp @@ -39,13 +39,13 @@ class Arbitrator : public Behavior { using ConstPtr = std::shared_ptr; /*! - * \brief The Option struct + * \brief The Option struct holds a behavior option of the arbitrator and corresponding flags * - * \todo explain why it is a subclass of arbitrator - * \todo explain flags implementation and usage - * - MyArbitrator::Option::Flags != Arbitrator::Option::Flags (no inheritance) - * - addOption() checks type, but hasFlag() not anymore (otherwise each inherited Option would have to - * implement a new, not overriding (because signature changed) hasFlag() -> error prone) + * This is a subclass of arbitrator, as some of the Flags could be arbitrator specific. + * For each arbitration class we have to make sure though, to derive an Option class from this Option base. + * + * \note When using addOption(), make sure to use the Flags of the correct Arbitrator. + * The compiler wouldn't notice a confusion unfortunately. */ struct Option { public: @@ -58,7 +58,6 @@ class Arbitrator : public Behavior { Option(const typename Behavior::Ptr& behavior, const FlagsT& flags) : behavior_{behavior}, flags_{flags} { } - //! \todo document why we need this virtual ~Option() = default; typename Behavior::Ptr behavior_; diff --git a/test/cost_arbitrator.cpp b/test/cost_arbitrator.cpp index 4cbacf70..9ff5b034 100644 --- a/test/cost_arbitrator.cpp +++ b/test/cost_arbitrator.cpp @@ -90,11 +90,11 @@ TEST_F(CostArbitratorTest, BasicFunctionality) { testCostArbitrator.gainControl(time); EXPECT_EQ("mid_cost", testCostArbitrator.getCommand(time)); - //! \todo This should be 0, if we estimate costs without calling getCommand (and thus gain/loseControl, see c2b2a93) + //! \note This should be 0, if we estimate costs without calling getCommand (and thus gain/loseControl, see c2b2a93) EXPECT_EQ(1, testBehaviorMidCost->loseControlCounter_); EXPECT_EQ("mid_cost", testCostArbitrator.getCommand(time)); - //! \todo This should be 1, if we estimate costs without calling getCommand (and thus gain/loseControl, see c2b2a93) + //! \note This should be 1, if we estimate costs without calling getCommand (and thus gain/loseControl, see c2b2a93) EXPECT_EQ(3, testBehaviorMidCost->loseControlCounter_); testBehaviorMidCost->invocationCondition_ = false; @@ -102,10 +102,10 @@ TEST_F(CostArbitratorTest, BasicFunctionality) { EXPECT_TRUE(testCostArbitrator.checkCommitmentCondition(time)); EXPECT_EQ("high_cost", testCostArbitrator.getCommand(time)); - //! \todo This should be 0, if we estimate costs without calling getCommand (and thus gain/loseControl, see c2b2a93) + //! \note This should be 0, if we estimate costs without calling getCommand (and thus gain/loseControl, see c2b2a93) EXPECT_EQ(3, testBehaviorHighCost->loseControlCounter_); EXPECT_EQ("high_cost", testCostArbitrator.getCommand(time)); - //! \todo This should be 0, if we estimate costs without calling getCommand (and thus gain/loseControl, see c2b2a93) + //! \note This should be 0, if we estimate costs without calling getCommand (and thus gain/loseControl, see c2b2a93) EXPECT_EQ(3, testBehaviorHighCost->loseControlCounter_); // high_cost behavior is not interruptable -> high_cost should stay active