Handle remaining todo code comments #patch

include/arbitration_graphs/arbitrator.hpp

@@ -39,13 +39,13 @@ class Arbitrator : public Behavior<CommandT> {
     using ConstPtr = std::shared_ptr<const Arbitrator>;
 
     /*!
-     * \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<CommandT> {
         Option(const typename Behavior<SubCommandT>::Ptr& behavior, const FlagsT& flags)
                 : behavior_{behavior}, flags_{flags} {
         }
-        //! \todo document why we need this
         virtual ~Option() = default;
 
         typename Behavior<SubCommandT>::Ptr behavior_;

test/cost_arbitrator.cpp

@@ -90,22 +90,22 @@ 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;
     EXPECT_TRUE(testCostArbitrator.checkInvocationCondition(time));
     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