Merge pull request #139 from AutoModality/AM-679/no-target

fix: no target boundaries BREAKING CHANGE: default removed from configureHzStats AM-679/no-target

Author: Aaron Roller

include/super_lib/am_life_cycle.h

@@ -164,22 +164,26 @@ class AMLifeCycle
      * Provide the target, which is the approximate value you expect to receive. The warnings and errors will be 
      * provided with tolerance on both sides of the target. 
      * 
-     * The default tolerance for warnings is 5% and for errors is 10%.  Override the specific values as desired.
+     * Configurations key use the stats short name.
      * 
-     * stats_short_name:
+     * setting a target will also set a min/max 5% warn and 10% error
+     * no target allows for just min or just max or both.
+     * 
+     * stats_target_sets_min_max:
+     *  hz:
+     *   target: 100 # sets min_error=90,min_warn=95,max_warn=105,max_error=110
+     * 
+     * stats_only_min:
      *  hz:
-     *   target: 50
      *   error:
-     *    min: 10
-     *    max: 100
+     *     min: 50
      *   warn:
-     *    min: 20
-     *    max: 90
+     *     min: 60
+     * 
      * 
      * @param stats to be configured
-     * @param target_default provided in code for the value that will be provided unless overridden in yamls
      * */
-    AMStatReset& configureHzStats(AMStatReset& stats, const int target_default);
+    AMStatReset& configureHzStats(AMStatReset& stats);
 
     virtual void heartbeatCB(const ros::TimerEvent& event);
 

rostest/hz_config/hz_config_rostest.cpp

@@ -5,60 +5,96 @@ class HzConfigTest : public RostestBase, am::AMLifeCycle
 {
 protected:
 
-  AMStatReset hz_stats_target_ten_ = AMStatReset("default_target_ten", "Test HZ");
-  AMStatReset hz_stats_target_one_ = AMStatReset("default_target_one", "Test HZ");
-  AMStatReset hz_stats_target_one_hundred_ = AMStatReset("default_target_one_hundred", "Test HZ");
-  AMStatReset hz_stats_config_target_ten_ = AMStatReset("config_target_ten", "Test HZ");
-  AMStatReset hz_stats_config_no_target_ = AMStatReset("config_no_target", "Test HZ");
-  
+  AMStatReset hz_stats_target_one_ = AMStatReset("target_one", "target_one");
+  AMStatReset hz_stats_target_ten_ = AMStatReset("target_ten", "target_ten");
+  AMStatReset hz_stats_target_one_hundred_ = AMStatReset("target_one_hundred", "target_one_hundred");
+  AMStatReset hz_stats_no_target_ = AMStatReset("no_target", "no_target");
+  AMStatReset hz_stats_all_provided_ = AMStatReset("all_provided", "all_provided");
+  AMStatReset hz_stats_target_one_hundred_min_error_10_ = AMStatReset("target_one_hundred_min_error_10", "target_one_hundred_min_error_10");
+  AMStatReset hz_stats_min_only_ = AMStatReset("min_only", "min_only");
+  AMStatReset hz_stats_max_only_ = AMStatReset("max_only", "max_only");
+
   HzConfigTest() : RostestBase() {
-    configureHzStats(hz_stats_target_ten_,10);
-    configureHzStats(hz_stats_target_one_,1);
-    configureHzStats(hz_stats_target_one_hundred_,100);
-    configureHzStats(hz_stats_config_target_ten_,999);
-    configureHzStats(hz_stats_config_no_target_,999);
+    configureHzStats(hz_stats_target_ten_);
+    configureHzStats(hz_stats_target_one_);
+    configureHzStats(hz_stats_target_one_hundred_);
+    configureHzStats(hz_stats_no_target_);
+    configureHzStats(hz_stats_all_provided_);
+    configureHzStats(hz_stats_target_one_hundred_min_error_10_);
+    configureHzStats(hz_stats_min_only_);
+    configureHzStats(hz_stats_max_only_);
   }
 
 };
 
-TEST_F(HzConfigTest, test_default_target_ten)
+TEST_F(HzConfigTest, target_ten)
 {
   ASSERT_EQ(8,hz_stats_target_ten_.getMinError());
   ASSERT_EQ(9,hz_stats_target_ten_.getMinWarn());
   ASSERT_EQ(11,hz_stats_target_ten_.getMaxWarn());
   ASSERT_EQ(12,hz_stats_target_ten_.getMaxError());
+  ASSERT_TRUE(hz_stats_target_ten_.isValidatingMax());
+  ASSERT_TRUE(hz_stats_target_ten_.isValidatingMin());
 }
-TEST_F(HzConfigTest, hz_stats_target_one_)
+TEST_F(HzConfigTest, target_one)
 {
   ASSERT_EQ(0,hz_stats_target_one_.getMinError()) << "Shouldn't go below zero";
   ASSERT_EQ(0,hz_stats_target_one_.getMinWarn()) << "shouldn't go below zero";
   ASSERT_EQ(2,hz_stats_target_one_.getMaxWarn());
   ASSERT_EQ(3,hz_stats_target_one_.getMaxError());
+  ASSERT_TRUE(hz_stats_target_one_.isValidatingMax());
+  ASSERT_TRUE(hz_stats_target_one_.isValidatingMin());
 }
 
+TEST_F(HzConfigTest, all_provided)
+{
+  ASSERT_EQ(11,hz_stats_all_provided_.getMinError());
+  ASSERT_EQ(22,hz_stats_all_provided_.getMinWarn());
+  ASSERT_EQ(33,hz_stats_all_provided_.getMaxWarn());
+  ASSERT_EQ(44,hz_stats_all_provided_.getMaxError());
+  ASSERT_TRUE(hz_stats_all_provided_.isValidatingMax());
+  ASSERT_TRUE(hz_stats_all_provided_.isValidatingMin());
+}
 
-TEST_F(HzConfigTest, hz_stats_config_target_ten_)
+TEST_F(HzConfigTest, no_target)
 {
-  ASSERT_EQ(8,hz_stats_config_target_ten_.getMinError());
-  ASSERT_EQ(9,hz_stats_config_target_ten_.getMinWarn());
-  ASSERT_EQ(11,hz_stats_config_target_ten_.getMaxWarn());
-  ASSERT_EQ(12,hz_stats_config_target_ten_.getMaxError());
+  ASSERT_FALSE(hz_stats_no_target_.isValidatingMax());
+  ASSERT_FALSE(hz_stats_no_target_.isValidatingMin());
 }
 
-TEST_F(HzConfigTest, config_no_target)
+TEST_F(HzConfigTest, min_only)
+{
+  ASSERT_EQ(77,hz_stats_min_only_.getMinError());
+  ASSERT_EQ(88,hz_stats_min_only_.getMinWarn());
+  ASSERT_FALSE(hz_stats_min_only_.isValidatingMax());
+  ASSERT_TRUE(hz_stats_min_only_.isValidatingMin());
+}
+TEST_F(HzConfigTest, max_only)
 {
-  ASSERT_EQ(8,hz_stats_config_no_target_.getMinError());
-  ASSERT_EQ(9,hz_stats_config_no_target_.getMinWarn());
-  ASSERT_EQ(11,hz_stats_config_no_target_.getMaxWarn());
-  ASSERT_EQ(12,hz_stats_config_no_target_.getMaxError());
+  ASSERT_EQ(63,hz_stats_max_only_.getMaxError());
+  ASSERT_EQ(51,hz_stats_max_only_.getMaxWarn());
+  ASSERT_TRUE(hz_stats_max_only_.isValidatingMax());
+  ASSERT_FALSE(hz_stats_max_only_.isValidatingMin());
 }
 
-TEST_F(HzConfigTest, default_target_one_hundred)
+TEST_F(HzConfigTest, target_one_hundred)
 {
   ASSERT_EQ(90,hz_stats_target_one_hundred_.getMinError());
   ASSERT_EQ(95,hz_stats_target_one_hundred_.getMinWarn());
   ASSERT_EQ(105,hz_stats_target_one_hundred_.getMaxWarn());
   ASSERT_EQ(110,hz_stats_target_one_hundred_.getMaxError());
+  ASSERT_TRUE(hz_stats_target_one_hundred_.isValidatingMax());
+  ASSERT_TRUE(hz_stats_target_one_hundred_.isValidatingMin());
+}
+
+TEST_F(HzConfigTest, target_one_hundred_min_error_10)
+{
+  ASSERT_EQ(10,hz_stats_target_one_hundred_min_error_10_.getMinError());
+  ASSERT_EQ(95,hz_stats_target_one_hundred_min_error_10_.getMinWarn());
+  ASSERT_EQ(105,hz_stats_target_one_hundred_min_error_10_.getMaxWarn());
+  ASSERT_EQ(110,hz_stats_target_one_hundred_min_error_10_.getMaxError());
+  ASSERT_TRUE(hz_stats_target_one_hundred_min_error_10_.isValidatingMax());
+  ASSERT_TRUE(hz_stats_target_one_hundred_min_error_10_.isValidatingMin());
 }
 
 int main(int argc, char** argv)

rostest/hz_config/hz_config_rostest.yaml

@@ -1,13 +1,45 @@
 hz_config_rostest:
-  config_target_ten:
+  target_one:
+    hz:
+      target: 1
+  target_ten:
     hz:
       target: 10
+  target_one_hundred:
+    hz:
+      target: 100
+
+  # no validation stat
+  no_target:
+    something_else: "doesn't matter"
+  
+  # no target, all boundaries declared
+  all_provided:
+    hz:
+      error:
+        min: 11
+        max: 44
+      warn:
+        min: 22
+        max: 33
 
-  config_no_target:
+  # prove the values are set by target, but override can happen
+  target_one_hundred_min_error_10:
+    hz:
+      target: 100
+      error:
+        min: 10
+
+  min_only:
+    hz:
+      error:
+        min: 77
+      warn:
+        min: 88
+  
+  max_only:
     hz:
       error:
-        min: 8
-        max: 12
+        max: 63
       warn:
-        min: 9
-        max: 11
+        max: 51

rostest/param/param_rostest.cpp

@@ -22,13 +22,13 @@ class ParamTest : public RostestBase, am::AMLifeCycle
 TEST_F(ParamTest, test_params)
 {
   ASSERT_EQ(relative_param_,"relative_value");
-  ASSERT_TRUE(relative_result_);
+  ASSERT_TRUE(relative_result_) << "true since it found a configuration";
 
   ASSERT_EQ(absolute_param_,"absolute_value");
-  ASSERT_TRUE(absolute_result_);
+  ASSERT_TRUE(absolute_result_) << "true since it found a configuration";
 
   ASSERT_EQ(default_param_,default_value_);
-  ASSERT_FALSE(default_result_);
+  ASSERT_FALSE(default_result_) << "false since no configuration found";
 
 }
 

src/super_lib/am_life_cycle.cpp

@@ -365,25 +365,53 @@ void AMLifeCycle::addStatistics(diagnostic_updater::DiagnosticStatusWrapper& dsw
   dsw.summary((uint8_t)status, "update");
 }
 
-AMStatReset& AMLifeCycle::configureHzStats(AMStatReset& stats, const int target_default)
-{
-    int hz_target;
-    int hz_min_error;
-    int hz_min_warn;
-    int hz_max_warn;
-    int hz_max_error;
+AMStatReset& AMLifeCycle::configureHzStats(AMStatReset& stats)
+{
+    int unassigned=UINT_MAX;
+    int hz_target = unassigned;
+    int hz_min_error =unassigned;
+    int hz_min_warn=unassigned;
+    int hz_max_warn=unassigned;
+    int hz_max_error=unassigned;
     const std::string prefix = stats.getShortName();
-    std::string hz_prefix=prefix + "/hz/";
-    param<int>(hz_prefix + "target", hz_target, target_default);
-    //give 5% tolerance in either direction for warning, 10% for error.  Override default values as desired
-    const int warning_offset = std::ceil(hz_target * 0.05);
-    const int error_offset =  2 * warning_offset;
-    //don't go  below zero because that doesn't make any sense for hz.
-    param<int>(hz_prefix + "error/min", hz_min_error, std::max(0,hz_target - error_offset));
-    param<int>(hz_prefix + "warn/min",  hz_min_warn,  std::max(0,hz_target - warning_offset));
-    param<int>(hz_prefix + "warn/max",  hz_max_warn,  hz_target + warning_offset);
-    param<int>(hz_prefix + "error/max", hz_max_error, hz_target + error_offset);
-    stats.setWarnError(hz_min_error, hz_min_warn, hz_max_warn, hz_max_error); 
+    const std::string hz_prefix=prefix + "/hz/";
+    const std::string target_key   =hz_prefix + "target";
+    const std::string error_min_key=hz_prefix + "error/min";
+    const std::string warn_min_key =hz_prefix + "warn/min";
+    const std::string warn_max_key =hz_prefix + "warn/max";
+    const std::string error_max_key=hz_prefix + "error/max";
+
+    //set all if target is provided
+    if(param<int>(target_key, hz_target, 0))
+    {
+      //give 5% tolerance in either direction for warning, 10% for error.  Override default values as desired
+      const int warning_offset = std::ceil(hz_target * 0.05);
+      const int error_offset =  2 * warning_offset;
+      //don't go  below zero because that doesn't make any sense for hz.
+      hz_min_error=std::max(0,hz_target - error_offset);
+      hz_min_warn=std::max(0,hz_target - warning_offset);
+      hz_max_warn=hz_target + warning_offset;
+      hz_max_error=hz_target + error_offset;
+      stats.setWarnError(hz_min_error, hz_min_warn, hz_max_warn, hz_max_error); 
+    }
+    //override individual boundary configs if provided
+    if(param<int>(error_min_key, hz_min_error, hz_min_error))
+    {
+      stats.setMinError(hz_min_error); 
+    }
+    if(param<int>(warn_min_key, hz_min_warn, hz_min_warn))
+    {
+      stats.setMinWarn(hz_min_warn); 
+    }
+    if(param<int>(warn_max_key, hz_max_warn, hz_max_warn))
+    {
+      stats.setMaxWarn(hz_max_warn); 
+    }
+    if(param<int>(error_max_key, hz_max_error, hz_max_error))
+    {
+      stats.setMaxError(hz_max_error); 
+    }
+
     return stats;       
 }
 void AMLifeCycle::sendNodeUpdate()