feat(updater)!: allow user to set key of key-value pair

bjv-capra · bjv-capra · commit 640f16c8fbe6 · 2022-11-26T12:27:31.000+01:00
Signed-off-by: Bart Jimenez Vera &lt;bjv@capra.ooo&gt;
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -17,8 +17,6 @@ jobs:
         include:
           - ros_distribution: "foxy"
             os: "ubuntu-20.04"
-          - ros_distribution: "galactic"
-            os: "ubuntu-20.04"
           - ros_distribution: "humble"
             os: "ubuntu-22.04"
           - ros_distribution: "rolling"
diff --git a/micro_ros_common_diagnostics/src/hwmonitor.c b/micro_ros_common_diagnostics/src/hwmonitor.c
@@ -20,12 +20,22 @@
 
 #include "micro_ros_diagnostic_updater/micro_ros_diagnostic_updater.h"
 
+// Variable value set in main() if passed as argument
+uint16_t task_id = 0;
+
 rcl_ret_t
-my_diagnostic_task(diagnostic_value_t * kv)
+my_diagnostic_task(
+  diagnostic_value_t values[MICRO_ROS_DIAGNOSTIC_UPDATER_MAX_VALUES_PER_TASK],
+  uint8_t * number_of_values)
 {
+  (void)number_of_values;
+  *number_of_values = 1;
+
+  values[0].key = task_id;
+
   // actual diagnostic task to be implemented
   rclc_diagnostic_value_set_level(
-    kv,
+    &values[0],
     micro_ros_diagnostic_msgs__msg__MicroROSDiagnosticStatus__STALE);
 
   return RCL_RET_OK;
@@ -35,7 +45,6 @@ int main(int argc, const char * argv[])
 {
   uint16_t hardware_id = 0;
   uint16_t updater_id = 0;
-  uint16_t task_id = 0;
   if (argc < 2) {
     printf("Need at least one argument: hardware ID. Optional: updater ID, task ID.\n");
     exit(1);
@@ -91,7 +100,7 @@ int main(int argc, const char * argv[])
     return -1;
   }
   diagnostic_task_t task;
-  rc = rclc_diagnostic_task_init(&task, hardware_id, updater_id, task_id, &my_diagnostic_task);
+  rc = rclc_diagnostic_task_init(&task, hardware_id, updater_id, &my_diagnostic_task);
   if (rc != RCL_RET_OK) {
     printf("Error in creating diagnostic task\n");
     return -1;
diff --git a/micro_ros_diagnostic_updater/example/example_processor_updater.c b/micro_ros_diagnostic_updater/example/example_processor_updater.c
@@ -25,14 +25,17 @@ static const int16_t PROCESSOR_ID = 17;
 // The hardware id
 static const int16_t PROCESSOR_SERIAL = 1001;
 // Task id
-static const int16_t PROCESSOR_TEMPERATURE_TASK_ID = 0;
+static const int16_t PROCESSOR_TEMPERATURE_KEY = 0;
 // Task id
-static const int16_t PROCESSOR_LOAD_TASK_ID = 1;
+static const int16_t PROCESSOR_LOAD_KEY = 1;
 
 rcl_ret_t
 my_diagnostic_temperature(diagnostic_value_t * values, uint8_t * number_of_values)
 {
   *number_of_values = 1;
+
+  // Set the key to get translation
+  values[0].key = PROCESSOR_TEMPERATURE_KEY;
   // Fake a temperature
   ++my_diagnostic_temp;
   if (my_diagnostic_temp > 99) {
@@ -55,6 +58,9 @@ rcl_ret_t
 my_diagnostic_load(diagnostic_value_t * values, uint8_t * number_of_values)
 {
   *number_of_values = 1;
+
+  // Set the key to get translation
+  values[0].key = PROCESSOR_LOAD_KEY;
   // Fake a processor load
   rclc_diagnostic_value_set_int(&values[0], my_diagnostic_temp / 2);
 
@@ -111,15 +117,15 @@ int main(int argc, const char * argv[])
   }
   diagnostic_task_t temperature_task;
   rc = rclc_diagnostic_task_init(
-    &temperature_task, PROCESSOR_SERIAL, PROCESSOR_ID, PROCESSOR_TEMPERATURE_TASK_ID,
+    &temperature_task, PROCESSOR_SERIAL, PROCESSOR_ID,
     &my_diagnostic_temperature);
   if (rc != RCL_RET_OK) {
     printf("Error in creating diagnostic task\n");
     return -1;
   }
   diagnostic_task_t load_task;
   rc = rclc_diagnostic_task_init(
-    &load_task, PROCESSOR_SERIAL, PROCESSOR_ID, PROCESSOR_LOAD_TASK_ID,
+    &load_task, PROCESSOR_SERIAL, PROCESSOR_ID,
     &my_diagnostic_load);
   if (rc != RCL_RET_OK) {
     printf("Error in creating diagnostic website checker\n");
diff --git a/micro_ros_diagnostic_updater/example/example_website_checker.c b/micro_ros_diagnostic_updater/example/example_website_checker.c
@@ -25,35 +25,43 @@ static int my_website_status = 0;
 // Hardware ID
 static const int16_t WEBSITE_SERIAL = 998;
 // Updater ID
-static const int16_t WEBSITE_ID = 0;
+static const uint16_t WEBSITE_ID = 0;
 // Task ID
-static const int16_t WEBSITE_STATUS_TASK_ID = 42;
+static const uint16_t WEBSITE_STATUS_TASK_ID = 42;
 
 
 rcl_ret_t
-my_diagnostic_website_check(diagnostic_value_t * kv)
+my_diagnostic_website_check(
+  diagnostic_value_t values[MICRO_ROS_DIAGNOSTIC_UPDATER_MAX_VALUES_PER_TASK],
+  uint8_t * number_of_values)
 {
+  // Cast to avoid warnings
+  (void)number_of_values;
+  *number_of_values = 1;
+
   ++my_diagnostic_status;
+
+  values[0].key = WEBSITE_STATUS_TASK_ID;
   if (my_diagnostic_status > 99) {
     my_diagnostic_status = 0;
   }
   if (my_diagnostic_status % 13 == 0) {
     my_website_status = 404;
     rclc_diagnostic_value_set_level(
-      kv,
+      &values[0],
       micro_ros_diagnostic_msgs__msg__MicroROSDiagnosticStatus__WARN);
   } else if (my_diagnostic_status % 17 == 0) {
     my_website_status = 500;
     rclc_diagnostic_value_set_level(
-      kv,
+      &values[0],
       micro_ros_diagnostic_msgs__msg__MicroROSDiagnosticStatus__ERROR);
   } else {
     my_website_status = 200;
     rclc_diagnostic_value_set_level(
-      kv,
+      &values[0],
       micro_ros_diagnostic_msgs__msg__MicroROSDiagnosticStatus__OK);
   }
-  rclc_diagnostic_value_lookup(kv, my_website_status);
+  rclc_diagnostic_value_lookup(&values[0], my_website_status);
 
   return RCL_RET_OK;
 }
@@ -102,7 +110,7 @@ int main(int argc, const char * argv[])
   }
   diagnostic_task_t task;
   rc = rclc_diagnostic_task_init(
-    &task, WEBSITE_SERIAL, WEBSITE_ID, WEBSITE_STATUS_TASK_ID,
+    &task, WEBSITE_SERIAL, WEBSITE_ID,
     &my_diagnostic_website_check);
   if (rc != RCL_RET_OK) {
     printf("Error in creating diagnostic task\n");
diff --git a/micro_ros_diagnostic_updater/include/micro_ros_diagnostic_updater/micro_ros_diagnostic_updater.h b/micro_ros_diagnostic_updater/include/micro_ros_diagnostic_updater/micro_ros_diagnostic_updater.h
@@ -36,7 +36,6 @@ typedef struct diagnostic_value_t
 
 typedef struct diagnostic_task_t
 {
-  int16_t id;
   uint8_t number_of_values;
   diagnostic_value_t values[MICRO_ROS_DIAGNOSTIC_UPDATER_MAX_VALUES_PER_TASK];
   int16_t hardware_id;
@@ -72,7 +71,6 @@ rclc_diagnostic_task_init(
   diagnostic_task_t * task,
   int16_t hardware_id,
   int16_t updater_id,
-  int16_t id,
   rcl_ret_t (* function)(
     diagnostic_value_t[MICRO_ROS_DIAGNOSTIC_UPDATER_MAX_VALUES_PER_TASK],
     uint8_t * number_of_values));
diff --git a/micro_ros_diagnostic_updater/src/micro_ros_diagnostic_updater/micro_ros_diagnostic_updater.c b/micro_ros_diagnostic_updater/src/micro_ros_diagnostic_updater/micro_ros_diagnostic_updater.c
@@ -53,7 +53,6 @@ rclc_diagnostic_task_init(
   diagnostic_task_t * task,
   int16_t hardware_id,
   int16_t updater_id,
-  int16_t id,
   rcl_ret_t (* function)(
     diagnostic_value_t[MICRO_ROS_DIAGNOSTIC_UPDATER_MAX_VALUES_PER_TASK],
     uint8_t * number_of_values))
@@ -63,7 +62,6 @@ rclc_diagnostic_task_init(
   RCL_CHECK_FOR_NULL_WITH_MSG(
     function, "function is a null pointer", return RCL_RET_INVALID_ARGUMENT);
 
-  task->id = id;
   task->function = function;
   task->updater_id = updater_id;
   task->hardware_id = hardware_id;
@@ -148,8 +146,7 @@ rclc_diagnostic_updater_add_task(
 
   if (updater->num_tasks >= MICRO_ROS_DIAGNOSTIC_UPDATER_MAX_TASKS_PER_UPDATER) {
     RCUTILS_LOG_ERROR(
-      "Updater could not add task %d, already %d tasks added.",
-      task->id,
+      "Updater could not add task, already %d tasks added.",
       MICRO_ROS_DIAGNOSTIC_UPDATER_MAX_TASKS_PER_UPDATER);
     return RCL_RET_ERROR;
   }
@@ -190,7 +187,7 @@ rclc_diagnostic_updater_update(
       for (uint8_t value_index = 0u; value_index < updater->tasks[i]->number_of_values;
         value_index++)
       {
-        key_value.key = updater->tasks[i]->id;
+        key_value.key = updater->tasks[i]->values[value_index].key;
         key_value.value_type = updater->tasks[i]->values[value_index].value_type;
         key_value.bool_value = updater->tasks[i]->values[value_index].bool_value;
         key_value.int_value = updater->tasks[i]->values[value_index].int_value;
@@ -205,13 +202,11 @@ rclc_diagnostic_updater_update(
       rcl_ret_t rc = rcl_publish(&updater->diag_pub, &updater->diag_status, NULL);
       if (rc == RCL_RET_OK) {
         RCUTILS_LOG_DEBUG(
-          "Updater published value for '%d'.",
-          updater->tasks[i]->id);
+          "Updater %d published value for '%d'.", updater->id, i);
       }
     } else {
       RCUTILS_LOG_ERROR(
-        "Updater could not update diagnostic task '%d'.",
-        updater->tasks[i]->id);
+        "Updater %d could not update diagnostic task '%d'.", updater->id, i);
     }
   }
 
diff --git a/micro_ros_diagnostic_updater/test/test_diagnostic_updater.cpp b/micro_ros_diagnostic_updater/test/test_diagnostic_updater.cpp
@@ -28,7 +28,9 @@ static int diagnostic_mockup_counter_0 = 0;
 static int diagnostic_mockup_counter_1 = 0;
 
 rcl_ret_t
-update_function_mockup_0(diagnostic_value_t * values, uint8_t * number_of_values)
+update_function_mockup_0(
+  diagnostic_value_t values[MICRO_ROS_DIAGNOSTIC_UPDATER_MAX_VALUES_PER_TASK],
+  uint8_t * number_of_values)
 {
   ++diagnostic_mockup_counter_0;
 
@@ -43,7 +45,9 @@ update_function_mockup_0(diagnostic_value_t * values, uint8_t * number_of_values
 }
 
 rcl_ret_t
-update_function_mockup_1(diagnostic_value_t * values, uint8_t * number_of_values)
+update_function_mockup_1(
+  diagnostic_value_t values[MICRO_ROS_DIAGNOSTIC_UPDATER_MAX_VALUES_PER_TASK],
+  uint8_t * number_of_values)
 {
   ++diagnostic_mockup_counter_1;
 
@@ -58,7 +62,7 @@ update_function_mockup_1(diagnostic_value_t * values, uint8_t * number_of_values
 
 TEST(TestDiagnosticUpdater, create_diagnostic_task) {
   diagnostic_task_t task;
-  rcl_ret_t rc = rclc_diagnostic_task_init(&task, 0, 0, 0, &update_function_mockup_0);
+  rcl_ret_t rc = rclc_diagnostic_task_init(&task, 0, 0, &update_function_mockup_0);
   EXPECT_EQ(RCL_RET_OK, rc);
 }
 
@@ -131,7 +135,7 @@ TEST(TestDiagnosticUpdater, updater_add_tasks) {
   EXPECT_EQ(RCL_RET_OK, rc);
 
   diagnostic_task_t task;
-  rc = rclc_diagnostic_task_init(&task, 17, 0, 0, &update_function_mockup_0);
+  rc = rclc_diagnostic_task_init(&task, 17, 0, &update_function_mockup_0);
   EXPECT_EQ(RCL_RET_OK, rc);
 
   rc = rclc_diagnostic_updater_add_task(&updater, &task);
@@ -171,11 +175,11 @@ TEST(TestDiagnosticUpdater, updater_update) {
   diagnostic_task_t task0, task1;
   rc = rclc_diagnostic_task_init(
     &task0,
-    0, 0, 0,
+    0, 0,
     &update_function_mockup_0);
   rc = rclc_diagnostic_task_init(
     &task1,
-    0, 0, 1,
+    0, 0,
     &update_function_mockup_1);
 
   rc = rclc_diagnostic_updater_add_task(&updater, &task0);
