CHG: code according to review

Author: slimasziom

robotiq_hande_driver/include/application.hpp

@@ -5,13 +5,14 @@
 
 #include "protocol_logic.hpp"
 
-/**
- * @brief This class contains high level gripper commands and status
- */
 
 namespace hande_driver
 {
 
+
+/**
+ * @brief This class contains high level gripper commands and status
+ */
 class ApplicationLayer{
 public:
 

robotiq_hande_driver/include/communication.hpp

@@ -1,38 +1,36 @@
 #ifndef COMMUNICATION_HPP_
 #define COMMUNICATION_HPP_
 
-#include <stdint.h>
-#include <stddef.h>
-#include <vector>
 #include <modbus/modbus.h>
 
-/**
- * @brief This class contains low level gripper commands and status
- */
 
 namespace hande_driver
 {
 constexpr auto kRegisterWordLength = 3;
 enum class OutputBytes : uint8_t {
-    OUTPUT_BYTES_RESERVED_1 = 0u,
-    OUTPUT_BYTES_ACTION_REQUEST,
-    OUTPUT_BYTES_POSITION_REQUEST,
-    OUTPUT_BYTES_RESERVED_2,
-    OUTPUT_BYTES_FORCE,
-    OUTPUT_BYTES_SPEED,
-    OUTPUT_BYTES_MAX
+    RESERVED_1 = 0u,
+    ACTION_REQUEST,
+    POSITION_REQUEST,
+    RESERVED_2,
+    FORCE,
+    SPEED,
+    BYTES_MAX
 };
 
 enum class InputBytes : uint8_t {
-    INPUT_BYTES_RESERVED_1 = 0u,
-    INPUT_BYTES_GRIPPER_STATUS,
-    INPUT_BYTES_POSITION_REQUEST_ECHO,
-    INPUT_BYTES_FAULT_STATUS,
-    INPUT_BYTES_CURRENT,
-    INPUT_BYTES_POSITION,
-    INPUT_BYTES_MAX
+    RESERVED_1 = 0u,
+    GRIPPER_STATUS,
+    POSITION_REQUEST_ECHO,
+    FAULT_STATUS,
+    CURRENT,
+    POSITION,
+    BYTES_MAX
 };
 
+
+/**
+ * @brief This class contains low level gripper commands and status
+ */
 class Communication{
 public:
     Communication();
@@ -116,8 +114,8 @@ class Communication{
 private:
     modbus_t *mb_;
 
-    uint8_t input_bytes_[INPUT_BYTES_MAX];
-    uint8_t output_bytes_[OUTPUT_BYTES_MAX];
+    uint8_t input_bytes_[static_cast<size_t>(InputBytes::BYTES_MAX)];
+    uint8_t output_bytes_[static_cast<size_t>(OutputBytes::BYTES_MAX)];
 
 };
 }   // namespace hande_driver

robotiq_hande_driver/include/protocol_logic.hpp

@@ -1,13 +1,8 @@
 #ifndef PROTOCOL_LOGIC_HPP_
 #define PROTOCOL_LOGIC_HPP_
 
-#include <stdint.h>
-
 #include <communication.hpp>
 
-/**
- * @brief This class contains protocol oriented functions and definitions
- */
 
 namespace hande_driver
 {
@@ -97,6 +92,10 @@ constexpr uint8_t kPositionByte = 4;
 // Current
 constexpr uint8_t kCurrentByte = 5;
 
+
+/**
+ * @brief This class contains protocol oriented functions and definitions
+ */
 class ProtocolLogic{
 public:
     ProtocolLogic();

robotiq_hande_driver/src/communication.cpp

@@ -1,7 +1,6 @@
 #include "communication.hpp"
 
 #include <cstdio>
-#include <stdint.h>
 #include <string.h>
 
 
@@ -19,8 +18,8 @@ constexpr uint16_t kGripperOutputFirstReg = 0x07D0;
 constexpr uint16_t kGripperInputFirstReg = 0x03E8;
 
 Communication::Communication()
-:    input_bytes_{0, 0, 0, 0, 0, 0}
-,    output_bytes_{0, 0, 0, 0, 0, 0}
+:    input_bytes_{}
+,    output_bytes_{}
 {
     printf("Communication constructor\n");
     mb_ = modbus_new_rtu(kDeviceName, kBaudrate, kParity, kDataBits, kStopBit);
@@ -82,16 +81,16 @@ void Communication::clear_output_bytes(){
 }
 
 uint8_t Communication::get_input_byte(InputBytes index){
-    return input_bytes_[index];
+    return input_bytes_[(uint)index];
 }
 
 void Communication::set_output_byte(OutputBytes index, uint8_t value){
-    output_bytes_[index] = value;
+    output_bytes_[(uint)index] = value;
 }
 
 void Communication::write_action_bit(uint8_t position_bit, bool value){
-    output_bytes_[OUTPUT_BYTES_ACTION_REQUEST] = bit_set_to(
-        output_bytes_[OUTPUT_BYTES_ACTION_REQUEST], position_bit, value);
+    output_bytes_[(uint)OutputBytes::ACTION_REQUEST] = bit_set_to(
+        output_bytes_[(uint)OutputBytes::ACTION_REQUEST], position_bit, value);
 }
 
 inline uint Communication::bit_set_to(uint number, uint n, bool x) {

…iq_hande_driver/src/communication_test.c …_hande_driver/src/communication_test.cpprobotiq_hande_driver/src/communication_test.c renamed to robotiq_hande_driver/src/communication_test.cpp robotiq_hande_driver/src/communication_test.c renamed to robotiq_hande_driver/src/communication_test.cpp

@@ -2,7 +2,7 @@
 #include <unistd.h>
 #include <modbus/modbus.h>
 
-#define SERVER_ID 0x09
+constexpr uint8_t SERVER_ID = 0x09;
 
 int main(void) {
   modbus_t *mb;

robotiq_hande_driver/src/protocol_logic.cpp

@@ -9,15 +9,15 @@ constexpr auto kGripperPositionOpenedThreshold = 230;
 constexpr auto kGripperPositionClosedThreshold = 13;
 
 ProtocolLogic::ProtocolLogic()
-:   status_(0)
-,   activation_status_(GRIPPER_RESET)
-,   action_status_(STOPPED)
-,   gripper_status_(NOT_USED)
-,   object_detection_status_(REQ_POS_NO_OBJECT)
-,   fault_status_(0)
-,   position_request_echo_(0)
-,   position_(0)
-,   current_(0)
+:   status_()
+,   activation_status_(ActivationStatus::GRIPPER_RESET)
+,   action_status_(ActionStatus::STOPPED)
+,   gripper_status_(GripperStatus::NOT_USED)
+,   object_detection_status_(ObjectDetectionStatus::REQ_POS_NO_OBJECT)
+,   fault_status_()
+,   position_request_echo_()
+,   position_()
+,   current_()
 {
     printf("ProtocolLogic constructor\n");
 }
@@ -29,22 +29,22 @@ ProtocolLogic::~ProtocolLogic(){
 void ProtocolLogic::reset(){
     communication_.clear_output_bytes();
 
-    communication_.write_action_bit(kActivatePositionByte, DEACTIVATE_GRIPPER);
+    communication_.write_action_bit(kActivatePositionByte, (bool)Activate::DEACTIVATE_GRIPPER);
 
     communication_.read_write_registers();
 }
 
 void ProtocolLogic::set(){
     communication_.clear_output_bytes();
 
-    communication_.write_action_bit(kActivatePositionByte, ACTIVATE_GRIPPER);
+    communication_.write_action_bit(kActivatePositionByte, (bool)Activate::ACTIVATE_GRIPPER);
 
     communication_.read_write_registers();
 }
 
 void ProtocolLogic::auto_release(){
-    communication_.write_action_bit(kAutomaticReleasePositionByte, EMERGENCY_AUTO_RELEASE);
-    communication_.write_action_bit(kAutoReleaseDirectionPositionByte, OPENING);
+    communication_.write_action_bit(kAutomaticReleasePositionByte, (bool)AutomaticRelease::EMERGENCY_AUTO_RELEASE);
+    communication_.write_action_bit(kAutoReleaseDirectionPositionByte, (bool)AutoReleaseDirection::OPENING);
 
     communication_.read_write_registers();
 }
@@ -55,38 +55,38 @@ void ProtocolLogic::activate(){
 }
 
 void ProtocolLogic::go_to(uint8_t position, uint8_t velocity, uint8_t force){
-    communication_.write_action_bit(kGoToPositionByte, GO_TO_REQ_POS);
+    communication_.write_action_bit(kGoToPositionByte, (bool)GoTo::GO_TO_REQ_POS);
 
-    communication_.set_output_byte(OUTPUT_BYTES_POSITION_REQUEST, position);
-    communication_.set_output_byte(OUTPUT_BYTES_SPEED, velocity);
-    communication_.set_output_byte(OUTPUT_BYTES_FORCE, force);
+    communication_.set_output_byte(OutputBytes::POSITION_REQUEST, position);
+    communication_.set_output_byte(OutputBytes::SPEED, velocity);
+    communication_.set_output_byte(OutputBytes::FORCE, force);
 
     communication_.read_write_registers();
 }
 
 void ProtocolLogic::stop(){
-    communication_.write_action_bit(kGoToPositionByte, STOP);
+    communication_.write_action_bit(kGoToPositionByte, (bool)GoTo::STOP);
 
     communication_.read_write_registers();
 }
 
 bool ProtocolLogic::is_reset(){
-    return (gripper_status_ == GRIPPER_IN_RESET &&
-            activation_status_ == GRIPPER_RESET);
+    return (gripper_status_ == GripperStatus::GRIPPER_IN_RESET &&
+            activation_status_ == ActivationStatus::GRIPPER_RESET);
 }
 
 bool ProtocolLogic::is_ready(){
-    return (gripper_status_ == ACTIVATION_COMPLETE &&
-            activation_status_ == GRIPPER_ACTIVATION);
+    return (gripper_status_ == GripperStatus::ACTIVATION_COMPLETE &&
+            activation_status_ == ActivationStatus::GRIPPER_ACTIVATION);
 }
 
 bool ProtocolLogic::is_moving(){
-    return (action_status_ == GO_TO_POSITION_REQUEST &&
-            object_detection_status_ == MOTION_NO_OBJECT);
+    return (action_status_ == ActionStatus::GO_TO_POSITION_REQUEST &&
+            object_detection_status_ == ObjectDetectionStatus::MOTION_NO_OBJECT);
 }
 
 bool ProtocolLogic::is_stopped(){
-    return object_detection_status_ != MOTION_NO_OBJECT;
+    return object_detection_status_ != ObjectDetectionStatus::MOTION_NO_OBJECT;
 }
 
 bool ProtocolLogic::is_closed(){
@@ -98,8 +98,8 @@ bool ProtocolLogic::is_opened(){
 }
 
 bool ProtocolLogic::obj_detected(){
-    return (object_detection_status_ == STOPPED_OPENING_DETECTED ||
-            object_detection_status_ == STOPPED_CLOSING_DETECTED);
+    return (object_detection_status_ == ObjectDetectionStatus::STOPPED_OPENING_DETECTED ||
+            object_detection_status_ == ObjectDetectionStatus::STOPPED_CLOSING_DETECTED);
 }
 
 uint8_t ProtocolLogic::get_reg_pos(){
@@ -117,7 +117,7 @@ uint8_t ProtocolLogic::get_current(){
 void ProtocolLogic::refresh_registers(){
     communication_.read_write_registers();
 
-    status_ = communication_.get_input_byte(INPUT_BYTES_GRIPPER_STATUS);
+    status_ = communication_.get_input_byte(InputBytes::GRIPPER_STATUS);
 
     activation_status_ = (ActivationStatus)(
         (status_>>kActivationStatusPositionByte) & kActivationStatusBits);
@@ -131,11 +131,11 @@ void ProtocolLogic::refresh_registers(){
     object_detection_status_ = (ObjectDetectionStatus)(
         (status_>>kObjectDetectionStatusPositionByte) & kObjectDetectionStatusBits);
 
-    fault_status_ = communication_.get_input_byte(INPUT_BYTES_FAULT_STATUS);
+    fault_status_ = communication_.get_input_byte(InputBytes::FAULT_STATUS);
     //To bo specified
 
-    position_request_echo_ = communication_.get_input_byte(INPUT_BYTES_POSITION_REQUEST_ECHO);
-    position_ = communication_.get_input_byte(INPUT_BYTES_POSITION);
-    current_ = communication_.get_input_byte(INPUT_BYTES_CURRENT);
+    position_request_echo_ = communication_.get_input_byte(InputBytes::POSITION_REQUEST_ECHO);
+    position_ = communication_.get_input_byte(InputBytes::POSITION);
+    current_ = communication_.get_input_byte(InputBytes::CURRENT);
 }
 }   // namespace hande_driver