Merge pull request #145 from AutoModality/AM-695/error-on-init

Aaron Roller · web-flow · commit e3219001ea54 · 2021-04-02T10:34:10.000-07:00
fix: error respected during init AM-695/error-on-init
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -221,6 +221,8 @@ if (CATKIN_ENABLE_TESTING)
                  error_forced 
                  error_status 
                  error_status_without_stats 
+                 error_terminal_before_config
+                 error_tolerant_before_config
                  hz_config
                  manual_to_disarming 
                  param
diff --git a/include/super_lib/am_life_cycle.h b/include/super_lib/am_life_cycle.h
@@ -31,6 +31,18 @@ class AMLifeCycle
 {
   public:
     static constexpr std::string_view BROADCAST_NODE_NAME = "";
+    
+    /**Specific parts of the lifecycle where nodes have responsibilities.*/
+    LifeCycleState getState() const;
+
+    /**Simple indication of health */
+    LifeCycleStatus getStatus() const;
+    
+    /** @brief string represenation of LifeCycleState*/
+    const std::string_view& getStateName();
+    
+    /** @brief string representation of LifeCycleStatus*/
+    const std::string_view& getStatusName();
 
   private:
     /* Variables to help seperate business logic from AMLifeCycle ROS */
@@ -201,14 +213,6 @@ class AMLifeCycle
 
     void lifecycleCB(const brain_box_msgs::LifeCycleCommand::ConstPtr msg);
 
-    /**Specific parts of the lifecycle where nodes have responsibilities.*/
-    LifeCycleState getState() const;
-    /**Simple indication of health */
-    LifeCycleStatus getStatus() const;
-    /** @brief string represenation of LifeCycleState*/
-    const std::string_view& getStateName();
-    /** @brief string representation of LifeCycleStatus*/
-    const std::string_view& getStatusName();
 
     double getThrottleS() const;
     void setThrottleS(const double throttleS);
diff --git a/include/super_lib/am_life_cycle_mediator.h b/include/super_lib/am_life_cycle_mediator.h
@@ -193,8 +193,12 @@ class AMLifeCycleMediator
 
     bool redundantShutdown(const AMLifeCycleMediator::LifeCycleInfo& info);
 
+    /** @brief return true if already in error state */
     bool redundantError(const AMLifeCycleMediator::LifeCycleInfo& info);
 
+    /** @brief return true if in UNCONFIGURED or CONFIGURING */
+    bool unconfigured(const AMLifeCycleMediator::LifeCycleInfo& info);
+
     bool illegalDestroy(const AMLifeCycleMediator::LifeCycleInfo& info);
 
   private:
diff --git a/rostest/error_terminal_before_config/error_terminal_before_config_rostest.cpp b/rostest/error_terminal_before_config/error_terminal_before_config_rostest.cpp
@@ -0,0 +1,35 @@
+#include <am_rostest_lib/am_rostest.h>
+
+/** Shows an error that happens upon construction will be reported as an ERROR during configuration. 
+ */
+class LifeCycleErrorTest : public RostestBase, public am::AMLifeCycle
+{
+protected:
+
+  LifeCycleErrorTest() : 
+    RostestBase() 
+  {
+      errorTerminal("forcing error during construction", "HW8S");
+  }
+
+
+  void onConfigure()
+  {
+    //do nothing...stay in configure
+  }
+};
+
+TEST_F(LifeCycleErrorTest, testStatus_Error)
+{
+  waitUntil(LifeCycleState::FINALIZED,"NAKW");
+  waitUntil(LifeCycleStatus::ERROR,"IUIU");
+  waitUntil(LifeCycleStatus::ERROR,"NQNE");
+}
+
+int main(int argc, char** argv)
+{
+  ::testing::InitGoogleTest(&argc, argv);
+  ros::init(argc, argv, ros::this_node::getName());
+
+  return RUN_ALL_TESTS();
+}
diff --git a/rostest/error_terminal_before_config/error_terminal_before_config_rostest.test b/rostest/error_terminal_before_config/error_terminal_before_config_rostest.test
@@ -0,0 +1,6 @@
+<launch>
+  <!-- include test file and pass in this test node name -->
+  <include file="$(find am_super)/rostest/am_super_rostest.test">
+    <arg name="test_name" value="error_terminal_before_config"/>
+  </include>
+</launch>
diff --git a/rostest/error_tolerant_before_config/error_tolerant_before_config_rostest.cpp b/rostest/error_tolerant_before_config/error_tolerant_before_config_rostest.cpp
@@ -0,0 +1,28 @@
+#include <am_rostest_lib/am_rostest.h>
+
+/** Shows an error that happens upon construction will be reported as an ERROR during configuration. 
+ */
+class LifeCycleErrorTest : public RostestBase, public am::AMLifeCycle
+{
+protected:
+
+  LifeCycleErrorTest() : 
+    RostestBase() 
+  {
+      errorTolerant("tolerant error during construction", "HW8S");
+  }
+};
+
+TEST_F(LifeCycleErrorTest, testStatus_Error)
+{
+  waitUntil(LifeCycleState::INACTIVE,"NAKW");
+  waitUntil(LifeCycleStatus::OK,"NQNE");
+}
+
+int main(int argc, char** argv)
+{
+  ::testing::InitGoogleTest(&argc, argv);
+  ros::init(argc, argv, ros::this_node::getName());
+
+  return RUN_ALL_TESTS();
+}
diff --git a/rostest/error_tolerant_before_config/error_tolerant_before_config_rostest.test b/rostest/error_tolerant_before_config/error_tolerant_before_config_rostest.test
@@ -0,0 +1,6 @@
+<launch>
+  <!-- include test file and pass in this test node name -->
+  <include file="$(find am_super)/rostest/am_super_rostest.test">
+    <arg name="test_name" value="error_tolerant_before_config"/>
+  </include>
+</launch>
diff --git a/rostest/platform_required_fail/platform_required_fail_rostest.cpp b/rostest/platform_required_fail/platform_required_fail_rostest.cpp
@@ -19,7 +19,7 @@ TEST_F(PlatformRequiredFailTest, requiredPlatformDifferentThanActual)
   ros::param::param<string>("/am_super/platform/actual", platform_actual_param, "missing");
   ASSERT_EQ(platform_actual_param,"not_test");
   
-  waitUntil(LifeCycleState::CONFIGURING,"UIYT");
+
   waitUntil(LifeCycleState::FINALIZED,"23SS");
   waitUntilMissionState(brain_box_msgs::VxState::SHUTDOWN,"HYUJ");
 }
diff --git a/src/super_lib/am_life_cycle.cpp b/src/super_lib/am_life_cycle.cpp
@@ -263,10 +263,10 @@ bool AMLifeCycle::withinConfigureTolerance()
 {
   bool tolerated = false;
   //outside of configuring, we have no tolerance
-  if(life_cycle_info_.state == LifeCycleState::CONFIGURING)
+  if(life_cycle_mediator_.unconfigured(life_cycle_info_))
   {
     ros::Duration duration_since_configure = ros::Time::now() - configure_start_time_;
-    if (duration_since_configure <= ros::Duration(configure_tolerance_s) )
+    if (life_cycle_info_.state == LifeCycleState::UNCONFIGURED || duration_since_configure <= ros::Duration(configure_tolerance_s) )
     {
       tolerated = true;
     }
diff --git a/src/super_lib/am_life_cycle_mediator.cpp b/src/super_lib/am_life_cycle_mediator.cpp
@@ -205,6 +205,19 @@ bool AMLifeCycleMediator::shutdown(const AMLifeCycleMediator::LifeCycleInfo& inf
     info.state == LifeCycleState::ACTIVE;
 }
 
+bool AMLifeCycleMediator::unconfigured(const AMLifeCycleMediator::LifeCycleInfo& info)
+{
+  switch (info.state)
+  {
+  case LifeCycleState::UNCONFIGURED:
+  case LifeCycleState::CONFIGURING:
+    return true;
+  default:
+    return false;
+  }
+}
+
+
 bool AMLifeCycleMediator::redundantShutdown(const AMLifeCycleMediator::LifeCycleInfo& info)
 {
   return info.state == LifeCycleState::SHUTTING_DOWN || 
@@ -213,8 +226,7 @@ bool AMLifeCycleMediator::redundantShutdown(const AMLifeCycleMediator::LifeCycle
 
 bool AMLifeCycleMediator::redundantError(const AMLifeCycleMediator::LifeCycleInfo& info)
 {
-  return info.state == LifeCycleState::ERROR_PROCESSING || info.state == LifeCycleState::FINALIZED ||
-    info.state == LifeCycleState::UNCONFIGURED;
+  return info.state == LifeCycleState::ERROR_PROCESSING || info.state == LifeCycleState::FINALIZED;
 }
 
 bool AMLifeCycleMediator::illegalDestroy(const AMLifeCycleMediator::LifeCycleInfo& info)
diff --git a/test/am_life_cycle_mediator_unit_tests.cpp b/test/am_life_cycle_mediator_unit_tests.cpp
@@ -65,6 +65,14 @@ class AMLifeCycleMediatorTest : public ::testing::Test
     info.state = state;
     EXPECT_EQ(life_cycle_mediator_.illegalDestroy(info), expected);
   }
+
+  bool EXPECT_EQ_UNCONFIGURED(const LifeCycleState& state, bool expected)
+  {
+    AMLifeCycleMediator::LifeCycleInfo info;
+    info.state = state;
+    EXPECT_EQ(life_cycle_mediator_.unconfigured(info), expected);
+  }
+
 };
 
 TEST_F(AMLifeCycleMediatorTest, statusError_LCStatusError)
@@ -287,7 +295,7 @@ TEST_F(AMLifeCycleMediatorTest, redundantError)
 {
   EXPECT_EQ_REDUNDANT_ERROR(LifeCycleState::ERROR_PROCESSING, true);
   EXPECT_EQ_REDUNDANT_ERROR(LifeCycleState::FINALIZED, true);
-  EXPECT_EQ_REDUNDANT_ERROR(LifeCycleState::UNCONFIGURED, true);
+  EXPECT_EQ_REDUNDANT_ERROR(LifeCycleState::UNCONFIGURED, false);
 
   EXPECT_EQ_REDUNDANT_ERROR(LifeCycleState::ACTIVATING, false);
   EXPECT_EQ_REDUNDANT_ERROR(LifeCycleState::ACTIVE, false);
@@ -299,6 +307,23 @@ TEST_F(AMLifeCycleMediatorTest, redundantError)
   EXPECT_EQ_REDUNDANT_ERROR(LifeCycleState::SHUTTING_DOWN, false);
 }
 
+
+TEST_F(AMLifeCycleMediatorTest, unconfigured)
+{
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::CONFIGURING, true);
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::UNCONFIGURED, true);
+
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::FINALIZED, false);
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::ERROR_PROCESSING, false);
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::ACTIVATING, false);
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::ACTIVE, false);
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::CLEANING_UP, false);
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::DEACTIVATING, false);
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::INACTIVE, false);
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::INVALID, false);
+  EXPECT_EQ_UNCONFIGURED(LifeCycleState::SHUTTING_DOWN, false);
+}
+
 TEST_F(AMLifeCycleMediatorTest, illegalDestroy)
 {
   EXPECT_EQ_ILLEGAL_DESTROY(LifeCycleState::FINALIZED, false);

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ TEST_F(PlatformRequiredFailTest, requiredPlatformDifferentThanActual)`
`19`	`19`	`ros::param::param<string>("/am_super/platform/actual", platform_actual_param, "missing");`
`20`	`20`	`ASSERT_EQ(platform_actual_param,"not_test");`
`21`	`21`
`22`		`- waitUntil(LifeCycleState::CONFIGURING,"UIYT");`
	`22`	`+`
`23`	`23`	`waitUntil(LifeCycleState::FINALIZED,"23SS");`
`24`	`24`	`waitUntilMissionState(brain_box_msgs::VxState::SHUTDOWN,"HYUJ");`
`25`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -263,10 +263,10 @@ bool AMLifeCycle::withinConfigureTolerance()`
`263`	`263`	`{`
`264`	`264`	`bool tolerated = false;`
`265`	`265`	`//outside of configuring, we have no tolerance`
`266`		`- if(life_cycle_info_.state == LifeCycleState::CONFIGURING)`
	`266`	`+ if(life_cycle_mediator_.unconfigured(life_cycle_info_))`
`267`	`267`	`{`
`268`	`268`	`ros::Duration duration_since_configure = ros::Time::now() - configure_start_time_;`
`269`		`- if (duration_since_configure <= ros::Duration(configure_tolerance_s) )`
	`269`	`+ if (life_cycle_info_.state == LifeCycleState::UNCONFIGURED \|\| duration_since_configure <= ros::Duration(configure_tolerance_s) )`
`270`	`270`	`{`
`271`	`271`	`tolerated = true;`
`272`	`272`	`}`