From 5ada3a2232e5fdb6cd0e793eb7f7f8bcb3e8bc7a Mon Sep 17 00:00:00 2001
From: Christian Dreher <c.dreher@kit.edu>
Date: Fri, 24 Nov 2023 13:20:53 +0100
Subject: [PATCH] refactor: Improve a few error messages and clang-format

---
 source/armarx/control/ethercat/Bus.cpp        |  2 +-
 source/armarx/control/ethercat/RTUnit.cpp     | 52 +++++--------------
 .../armarx/control/ethercat/bus_io/BusIO.cpp  |  4 +-
 3 files changed, 16 insertions(+), 42 deletions(-)

diff --git a/source/armarx/control/ethercat/Bus.cpp b/source/armarx/control/ethercat/Bus.cpp
index b58c392c..55272065 100644
--- a/source/armarx/control/ethercat/Bus.cpp
+++ b/source/armarx/control/ethercat/Bus.cpp
@@ -1129,7 +1129,7 @@ namespace armarx::control::ethercat
             ARMARX_IMPORTANT << "ec_init(" << ifname << ")";
             if (!ec_init(ifname.c_str()))
             {
-                BUS_ERROR("Could not init etherCAT on %s\nExecute as root\n", ifname.c_str());
+                BUS_ERROR("Could not init EtherCAT on %s\nExecute as root\n", ifname.c_str());
                 return false;
             }
         }
diff --git a/source/armarx/control/ethercat/RTUnit.cpp b/source/armarx/control/ethercat/RTUnit.cpp
index f4d2a296..c95f3307 100644
--- a/source/armarx/control/ethercat/RTUnit.cpp
+++ b/source/armarx/control/ethercat/RTUnit.cpp
@@ -54,19 +54,16 @@
 #include <armarx/control/ethercat/SlaveErrorRegistersDevice.h>
 #include <armarx/control/ethercat/SlaveInterface.h>
 
-
 namespace armarx::control::ethercat
 {
     // The maximum stack size which is guaranteed safe to access without faulting
     static constexpr unsigned int MAX_SAFE_STACK = (8 * 1024);
 
-
     RTUnit::RTUnit()
     {
         ;
     }
 
-
     void
     RTUnit::onInitRobotUnit()
     {
@@ -112,7 +109,6 @@ namespace armarx::control::ethercat
         bus.setLocalMinimumLoggingLevel(this->getEffectiveLoggingLevel());
     }
 
-
     void
     RTUnit::onConnectRobotUnit()
     {
@@ -125,14 +121,12 @@ namespace armarx::control::ethercat
         }
     }
 
-
     void
     RTUnit::onDisconnectRobotUnit()
     {
         ARMARX_INFO << "RTUnit disconnects now";
     }
 
-
     void
     RTUnit::onExitRobotUnit()
     {
@@ -143,7 +137,6 @@ namespace armarx::control::ethercat
         ARMARX_INFO << "RTUnit exit complete";
     }
 
-
     void
     RTUnit::initializeKinematicUnit()
     {
@@ -169,7 +162,6 @@ namespace armarx::control::ethercat
         }
     }
 
-
     void
     RTUnit::startRTThread()
     {
@@ -195,7 +187,6 @@ namespace armarx::control::ethercat
                              }};
     }
 
-
     void
     RTUnit::joinControlThread()
     {
@@ -205,14 +196,12 @@ namespace armarx::control::ethercat
         ARMARX_INFO << "rt task stopped";
     }
 
-
     void
     RTUnit::publish(StringVariantBaseMap debugObserverMap, StringVariantBaseMap timingMap)
     {
         RobotUnit::publish(std::move(debugObserverMap), std::move(timingMap));
     }
 
-
     armarx::PropertyDefinitionsPtr
     RTUnit::createPropertyDefinitions()
     {
@@ -277,7 +266,6 @@ namespace armarx::control::ethercat
         return def;
     }
 
-
     RTUnit::~RTUnit()
     {
         ARMARX_INFO << "DTOR of RTUnit";
@@ -289,7 +277,6 @@ namespace armarx::control::ethercat
         }
     }
 
-
     void
     RTUnit::rtRun()
     {
@@ -323,7 +310,7 @@ namespace armarx::control::ethercat
         }
         catch (...)
         {
-            ARMARX_ERROR << "init ethercat bus failed with exception: "
+            ARMARX_ERROR << "Initializing EtherCAT bus RT thread failed with exception: "
                          << armarx::GetHandledExceptionString();
             return;
         }
@@ -420,7 +407,6 @@ namespace armarx::control::ethercat
         }
     }
 
-
     MultiNodeRapidXMLReader
     RTUnit::ReadHardwareConfigFile(std::string busConfigFilePath, std::string robotName)
     {
@@ -476,7 +462,6 @@ namespace armarx::control::ethercat
         return multiNode;
     }
 
-
     bool
     RTUnit::initBusRTThread()
     {
@@ -486,7 +471,7 @@ namespace armarx::control::ethercat
         bus.setIfName(properties.ethercatInterfaceName);
         if (!bus.switchBusToSafeOp())
         {
-            ARMARX_ERROR << "Starting bus to SafeOp failed.";
+            ARMARX_ERROR << "Bringing bus to EtherCAT state 'safe-operational' failed.";
             return false;
         }
 
@@ -537,12 +522,10 @@ namespace armarx::control::ethercat
             }
         }
 
-
         ARMARX_INFO << "EtherCAT bus is started";
         return true;
     }
 
-
     void
     RTUnit::controlLoopRTThread()
     {
@@ -565,8 +548,8 @@ namespace armarx::control::ethercat
             ARMARX_CHECK_EXPRESSION(rtRobotJointSet->getSize() > 0);
             VirtualRobot::Gravity gravity(rtGetRobot(), rtRobotJointSet, rtRobotBodySet);
 
-            const std::uint64_t loopDurationThreshold = properties.rtLoopTimeInUS / 1000 *
-            properties.rTLoopTimingCheckToleranceFactor;
+            const std::uint64_t loopDurationThreshold =
+                properties.rtLoopTimeInUS / 1000 * properties.rTLoopTimingCheckToleranceFactor;
 
             //#if 0
             std::vector<float> gravityValues(rtRobotJointSet->getSize());
@@ -576,7 +559,7 @@ namespace armarx::control::ethercat
 
             if (!bus.switchBusToOp())
             {
-                BUS_FATAL_AND_THROW("Switching bus to ESM state 'operational' failed");
+                BUS_FATAL_AND_THROW("Switching bus to EtherCAT state 'operational' failed");
             }
 
 
@@ -609,27 +592,21 @@ namespace armarx::control::ethercat
                     RT_TIMING_START(RunControllers)
                     RT_TIMING_START(SwitchControllers)
                     rtSwitchControllerSetup();
-                    RT_TIMING_CEND(SwitchControllers,
-                                   0.3 * loopDurationThreshold)
+                    RT_TIMING_CEND(SwitchControllers, 0.3 * loopDurationThreshold)
                     RT_TIMING_START(ResettingTargets)
                     rtResetAllTargets();
-                    RT_TIMING_CEND(ResettingTargets,
-                                   0.3 * loopDurationThreshold)
+                    RT_TIMING_CEND(ResettingTargets, 0.3 * loopDurationThreshold)
                     RT_TIMING_START(RunNJointControllers)
                     rtRunNJointControllers(currentPDOUpdateTimestamp, cappedDeltaT);
-                    RT_TIMING_CEND(RunNJointControllers,
-                                   0.3 * loopDurationThreshold)
+                    RT_TIMING_CEND(RunNJointControllers, 0.3 * loopDurationThreshold)
                     RT_TIMING_START(CheckInvalidTargets)
                     rtHandleInvalidTargets();
-                    RT_TIMING_CEND(CheckInvalidTargets,
-                                   0.3 * loopDurationThreshold)
+                    RT_TIMING_CEND(CheckInvalidTargets, 0.3 * loopDurationThreshold)
 
                     RT_TIMING_START(RunJointControllers)
                     rtRunJointControllers(currentPDOUpdateTimestamp, cappedDeltaT);
-                    RT_TIMING_CEND(RunJointControllers,
-                                   0.3 * loopDurationThreshold)
-                    RT_TIMING_CEND(RunControllers,
-                                   0.3 * loopDurationThreshold)
+                    RT_TIMING_CEND(RunJointControllers, 0.3 * loopDurationThreshold)
+                    RT_TIMING_CEND(RunControllers, 0.3 * loopDurationThreshold)
                 }
 
                 //bus update
@@ -682,8 +659,7 @@ namespace armarx::control::ethercat
 
                     i++;
                 }
-                RT_TIMING_CEND(ComputeGravityTorques,
-                               0.2 * loopDurationThreshold)
+                RT_TIMING_CEND(ComputeGravityTorques, 0.2 * loopDurationThreshold)
 
 
                 rtGetRTThreadTimingsSensorDevice().rtMarkRtLoopPreSleep();
@@ -691,8 +667,7 @@ namespace armarx::control::ethercat
                 const auto loopPreSleepTime = armarx::rtNow();
                 RT_TIMING_START(RTLoopWaiting)
                 auto diff = cycleKeeper.waitForCycleDuration();
-                RT_TIMING_CEND(RTLoopWaiting,
-                               loopDurationThreshold)
+                RT_TIMING_CEND(RTLoopWaiting, loopDurationThreshold)
                 const auto loopPostSleepTime = armarx::rtNow();
 
                 const auto loopDuration = armarx::rtNow() - loopStartTime;
@@ -809,7 +784,6 @@ namespace armarx::control::ethercat
         ARMARX_INFO << "Leaving control loop function";
     }
 
-
     void
     RTUnit::startBackupLogging()
     {
diff --git a/source/armarx/control/ethercat/bus_io/BusIO.cpp b/source/armarx/control/ethercat/bus_io/BusIO.cpp
index 745e3c76..4055de37 100644
--- a/source/armarx/control/ethercat/bus_io/BusIO.cpp
+++ b/source/armarx/control/ethercat/bus_io/BusIO.cpp
@@ -62,7 +62,7 @@ namespace armarx::control::ethercat
     {
         if (!sdoAccessAvailable)
         {
-            BUS_WARNING("SDO-access is not yet available. Probably the bus was not started yet");
+            BUS_WARNING("SDO-access is not yet available.  Probably the bus was not started yet");
             return false;
         }
         return true;
@@ -227,7 +227,7 @@ namespace armarx::control::ethercat
                         actualState.c_str(),
                         timeouts.stateCheck_us,
                         statecheckDuration.toMicroSeconds(),
-                        probablyTimedOut ? ", so the operation probably timed out." : "");
+                        probablyTimedOut ? ", so the operation probably timed out" : "");
                     request->setFailed();
                 }
 
-- 
GitLab