refs #PMP2-305 Added logic to reject information messages with invali…

include/hnzpath.h

@@ -89,7 +89,10 @@ class HNZPath {
    * Is the HNZ connection with the PA established and still alive?
    * @return true if connected, false otherwise
    */
-  bool isHNZConnected() { return (m_protocol_state == CONNECTED) && isConnected(); };
+  bool isHNZConnected() {
+    std::lock_guard<std::recursive_mutex> lock(m_protocol_state_mutex);
+    return (m_protocol_state == CONNECTED) && isConnected();
+  };
 
   /**
    * Is the TCP connection with the PA established and still alive?
@@ -161,7 +164,10 @@ class HNZPath {
    * Gets the state of the HNZ protocol (CONNECTION, CONNECTED)
    * @return CONNECTION if SARM/UA step is not complete, CONNECTED after that
    */
-  int getProtocolState() const { return m_protocol_state; }
+  int getProtocolState() const { 
+    std::lock_guard<std::recursive_mutex> lock(m_protocol_state_mutex);
+    return m_protocol_state;
+  }
 
  private:
   std::unique_ptr<HNZClient> m_hnz_client;  // HNZ Client that manage TCP connection
@@ -173,54 +179,55 @@ class HNZPath {
   list<Command_message>
       command_sent;  // List of command already sent waiting to be ack
 
-  long last_sent_time;     // Timestamp of the last message sent
-  int repeat_max;          // max number of authorized repeats
+  long last_sent_time = 0;     // Timestamp of the last message sent
+  int repeat_max = 0;          // max number of authorized repeats
   int gi_repeat = 0;       // number of time a GI is repeated
   long gi_start_time = 0;  // GI start time
 
   std::shared_ptr<std::thread> m_connection_thread; // Main thread that maintains the connection
+  std::mutex m_connection_thread_mutex; // mutex to protect changes in m_connection_thread
   atomic<bool> m_is_running{true};  // If false, the connection thread will stop
   atomic<bool> m_connected{false};  // TCP Connection state with the PA
   // Initializing to CONNECTED ensures that the initial state transition from go_to_connection generates an audit
   int m_protocol_state = CONNECTED; // HNZ Protocol connection state
+  mutable std::recursive_mutex m_protocol_state_mutex; // mutex to protect changes in m_protocol_state
   bool m_is_active_path = false;
 
   // Plugin configuration
   string m_ip;  // IP of the PA
-  int m_port;   // Port to connect to
-  long long int m_timeoutUs; // Timeout for socket recv in microseconds
+  int m_port = 0;   // Port to connect to
+  long long int m_timeoutUs = 0; // Timeout for socket recv in microseconds
 
   string m_name_log;   // Path name used in log
   string m_path_letter; // Path letter
   string m_path_name;  // Path name
 
-  unsigned int m_remote_address;
-  unsigned char m_address_PA;   // remote address + 1
-  unsigned char m_address_ARP;  // remote address + 3
+  unsigned int m_remote_address = 0;
+  unsigned char m_address_PA = 0;   // remote address + 1
+  unsigned char m_address_ARP = 0;  // remote address + 3
 
-  int m_max_sarm;  // max number of SARM messages before handing over to the
+  int m_max_sarm = 0;  // max number of SARM messages before handing over to the
                    // passive path
-  int m_inacc_timeout;   // timeout before declaring the remote server
+  int m_inacc_timeout = 0;   // timeout before declaring the remote server
                          // unreachable
-  int m_repeat_timeout;  // time allowed for the receiver to acknowledge a frame
-  int m_anticipation_ratio;  // number of frames allowed to be received without
+  int m_repeat_timeout = 0;  // time allowed for the receiver to acknowledge a frame
+  int m_anticipation_ratio = 0;  // number of frames allowed to be received without
                              // acknowledgement
   BulleFormat m_test_msg_receive;  // Payload of received BULLE
   BulleFormat m_test_msg_send;     // Payload of sent BULLE
-  int c_ack_time_max;  // Max time to wait before receving a acknowledgement for
+  int c_ack_time_max = 0;  // Max time to wait before receving a acknowledgement for
                       // a control command (in ms)
 
   // HNZ protocol related variable
-  int m_nr;   // Number in reception
-  int m_ns;   // Number in sending
-  int m_NRR;  // Received aquit number
-  int module10M;
-  long m_last_msg_time;   // Timestamp of the last reception
-  bool sarm_PA_received;  // The SARM sent by the PA was received
-  bool sarm_ARP_UA;     // The UA sent by the PA (after receiving our SARM) was
+  int m_nr = 0;   // Number in reception
+  int m_ns = 0;   // Number in sending
+  int m_NRR = 0;  // Received aquit number
+  long m_last_msg_time = 0;   // Timestamp of the last reception
+  bool sarm_PA_received = false;  // The SARM sent by the PA was received
+  bool sarm_ARP_UA = false;     // The UA sent by the PA (after receiving our SARM) was
                         // received
-  int m_nbr_sarm_sent;  // Number of SARM sent
-  int m_repeat;         // Number of times the sent message is repeated
+  int m_nbr_sarm_sent = 0;  // Number of SARM sent
+  int m_repeat = 0;         // Number of times the sent message is repeated
 
   /**
    * Manage the HNZ protocol connection with the PA. Be careful, it doesn't
@@ -294,15 +301,17 @@ class HNZPath {
    * Call this method when a RR message is received.
    * @param nr NR of the RTU
    * @param repetition set to true if frame received is repeated
+   * @return True if the NR contained in the message was correct, else false
    */
-  void m_receivedRR(int nr, bool repetition);
+  bool m_receivedRR(int nr, bool repetition);
 
   /**
    * Send a RR
    * @param repetition set to true if frame received is repeated
    * @param ns NS of the received frame
+   * @return True if received NR was valid and RR was sent, false if invalid NR was received and no RR was sent
    */
-  void m_sendRR(bool repetition, int ns, int nr);
+  bool m_sendRR(bool repetition, int ns, int nr);
 
   /**
    * Send an information frame. The address byte, numbering bit (containing NR,
@@ -356,6 +365,19 @@ class HNZPath {
    * @param beforeLog Prefix for the log messages produced by this function
    */
   void m_registerCommandIfSent(const std::string& type, bool sent, unsigned char address, int value, const std::string& beforeLog);
+
+  /**
+   * Test if a NR is valid
+   * @param nr NR of the RTU
+   * @return True if the NR contained in the message was correct, else false
+   */
+  bool m_isNRValid(int nr) const;
+
+  /**
+   * Called to update internal values once a message containing a valid NR was received
+   * @param nr NR of the RTU
+   */
+  void m_NRAccepted(int nr);
 };
 
 #endif

src/hnz.cpp

@@ -168,8 +168,7 @@ bool HNZ::setJsonConfig(const string& protocol_conf_json, const string& msg_conf
 }
 
 void HNZ::receive(std::shared_ptr<HNZPath> hnz_path_in_use) {
-  if (m_hnz_conf) {
-    // Parent if used only for scope lock
+  {
     std::lock_guard<std::recursive_mutex> guard(m_configMutex);
     if (!m_hnz_conf->is_complete()) {
       return;
@@ -606,9 +605,7 @@ bool HNZ::operation(const std::string& operation, int count, PLUGIN_PARAMETER**
   std::string beforeLog = HnzUtility::NamePlugin + " - HNZ::operation -";
   HnzUtility::log_info("%s Operation %s: %s", beforeLog.c_str(), operation.c_str(), paramsToStr(params, count).c_str());
 
-  // Workaround until the following ticket is fixed: https://github.com/fledge-iot/fledge/issues/1239
-  // if (operation == "HNZCommand") {
-  if (endsWith(operation, "Command")) {
+  if (operation == "HNZCommand") {
     int res = processCommandOperation(count, params);
     if(res == 0) {
       // Only return on success so that all parameters are displayed by final error log in case of syntax error
@@ -644,10 +641,6 @@ int HNZ::processCommandOperation(int count, PLUGIN_PARAMETER** params) {
     const std::string& paramValue = params[i]->value;
     if (commandParams.count(paramName) > 0) {
       commandParams[paramName] = paramValue;
-      // Workaround until the following ticket is fixed: https://github.com/fledge-iot/fledge/issues/1240
-      if(paramValue.at(0) == '"'){
-        commandParams[paramName] = paramValue.substr(1,paramValue.length()-2);
-      }
     }
     else {
       HnzUtility::log_warn("%s Unknown parameter '%s' in HNZCommand", beforeLog.c_str(), paramName.c_str());

src/hnzconnection.cpp

@@ -71,8 +71,7 @@ void HNZConnection::stop() {
 
   // Stop the path used (close the TCP connection and stop the threads that
   // manage HNZ connections)
-  if (m_active_path != nullptr || m_passive_path != nullptr) {
-    // Parent if is mostly here for scope lock
+  {
     std::lock_guard<std::recursive_mutex> lock(m_path_mutex);
     if (m_active_path != nullptr) m_active_path->disconnect();
     if (m_passive_path != nullptr) m_passive_path->disconnect();
@@ -90,8 +89,8 @@ void HNZConnection::stop() {
 }
 
 void HNZConnection::checkGICompleted(bool success) { 
-  std::lock_guard<std::recursive_mutex> lock(m_path_mutex);
   std::string beforeLog = HnzUtility::NamePlugin + " - HNZConnection::checkGICompleted -";
+  std::lock_guard<std::recursive_mutex> lock(m_path_mutex);
 
   // GI is a success
   if (success) {
@@ -140,8 +139,7 @@ void HNZConnection::m_manageMessages() {
     m_update_current_time();
 
     // Manage repeat/timeout for each path
-    if (m_active_path || m_passive_path) {
-      // Parent if is mostly here for scope lock
+    {
       std::lock_guard<std::recursive_mutex> lock(m_path_mutex);
       m_check_timer(m_active_path);
       m_check_timer(m_passive_path);