From 0a4b3f9204ec93be22dc25960679058d8bdcb065 Mon Sep 17 00:00:00 2001
From: Jacob Boddey <boddey@google.com>
Date: Fri, 9 Feb 2024 08:39:43 +0000
Subject: [PATCH 1/4] Re-enable protocol tests

---
 modules/test/protocol/conf/module_config.json |  6 ++---
 .../protocol/python/src/protocol_bacnet.py    | 26 ++++++++++---------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/modules/test/protocol/conf/module_config.json b/modules/test/protocol/conf/module_config.json
index 0fa83afb8..a51a3603b 100644
--- a/modules/test/protocol/conf/module_config.json
+++ b/modules/test/protocol/conf/module_config.json
@@ -1,6 +1,6 @@
 {
   "config": {
-    "enabled": false,
+    "enabled": true,
     "meta": {
       "name": "protocol",
       "display_name": "Protocol",
@@ -17,13 +17,13 @@
         "name": "protocol.valid_bacnet",
         "test_description": "Can valid BACnet traffic be seen",
         "expected_behavior": "BACnet traffic can be seen on the network and packets are valid and not malformed",
-        "required_result": "Required"
+        "required_result": "Recommended"
       },
       {
         "name": "protocol.valid_modbus",
         "test_description": "Can valid Modbus traffic be seen",
         "expected_behavior": "Any Modbus functionality works as expected and valid modbus traffic can be observed",
-        "required_result": "Required",
+        "required_result": "Recommended",
         "config":{
           "port": 502,
           "device_id": 1,
diff --git a/modules/test/protocol/python/src/protocol_bacnet.py b/modules/test/protocol/python/src/protocol_bacnet.py
index 557dcdfb9..c23061c51 100644
--- a/modules/test/protocol/python/src/protocol_bacnet.py
+++ b/modules/test/protocol/python/src/protocol_bacnet.py
@@ -11,7 +11,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-"""Module run all the BACnet related methods for testing"""
+"""Module to run all the BACnet related methods for testing"""
 
 import BAC0
 import logging
@@ -28,39 +28,41 @@ def __init__(self, log):
     LOGGER = log
 
     # Setup the BAC0 Log
-    BAC0.log_level(log_file=logging.DEBUG, stdout=logging.INFO, stderr=logging.CRITICAL)
-    
+    BAC0.log_level(log_file=logging.DEBUG,
+                   stdout=logging.INFO,
+                   stderr=logging.CRITICAL)
+
     self.devices = []
 
   def discover(self, local_ip=None):
-    LOGGER.info("Performing BACnet discovery...")
+    LOGGER.info('Performing BACnet discovery...')
     bacnet = BAC0.lite(local_ip)
-    LOGGER.info("Local BACnet object: " + str(bacnet))
+    LOGGER.info('Local BACnet object: ' + str(bacnet))
     try:
       bacnet.discover(global_broadcast=True)
     except Exception as e:
       LOGGER.error(e)
-    LOGGER.info("BACnet discovery complete")
+    LOGGER.info('BACnet discovery complete')
     with open(BAC0_LOG,'r',encoding='utf-8') as f:
       bac0_log = f.read()
-    LOGGER.info("BAC0 Log:\n" + bac0_log)
+    LOGGER.info('BAC0 Log:\n' + bac0_log)
     self.devices = bacnet.devices
 
   # Check if the device being tested is in the discovered devices list
   def validate_device(self, local_ip, device_ip):
     result = None
-    LOGGER.info("Validating BACnet device: " + device_ip)
+    LOGGER.info('Validating BACnet device: ' + device_ip)
     self.discover(local_ip + '/24')
-    LOGGER.info("BACnet Devices Found: " + str(len(self.devices)))
+    LOGGER.info('BACnet Devices Found: ' + str(len(self.devices)))
     if len(self.devices) > 0:
       # Load a fail result initially and pass only
       # if we can validate it's the right device responding
       result = False, (
-        f'Could not confirm discovered BACnet device is the ' +
+        'Could not confirm discovered BACnet device is the ' +
         'same as device being tested')
       for device in self.devices:
-        name, vendor, address, device_id = device
-        LOGGER.info("Checking Device: " + str(device))
+        address = device[2]
+        LOGGER.info('Checking device: ' + str(device))
         if device_ip in address:
           result = True, 'Device IP matches discovered device'
           break

From bba91e8c491828b623bf9dd03fc47f359761d3f3 Mon Sep 17 00:00:00 2001
From: jhughesbiot <jonathan.hughes@buildingsiot.com>
Date: Wed, 28 Feb 2024 09:58:39 -0700
Subject: [PATCH 2/4] Reorder protocol module run order

---
 framework/python/src/test_orc/test_orchestrator.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/framework/python/src/test_orc/test_orchestrator.py b/framework/python/src/test_orc/test_orchestrator.py
index a81fca4fd..311e57316 100644
--- a/framework/python/src/test_orc/test_orchestrator.py
+++ b/framework/python/src/test_orc/test_orchestrator.py
@@ -462,7 +462,14 @@ def _load_test_modules(self):
     loaded_modules = "Loaded the following test modules: "
     test_modules_dir = os.path.join(self._path, TEST_MODULES_DIR)
 
-    for module_dir in os.listdir(test_modules_dir):
+    module_dirs = os.listdir(test_modules_dir)
+    # Check if the directory protocol exists and move it to the beginning
+    # protocol should always be run first so BACnet binding doesn't get
+    # corrupted during DHCP changes in the conn module
+    if 'protocol' in module_dirs:
+      module_dirs.insert(0, module_dirs.pop(module_dirs.index('protocol')))
+
+    for module_dir in module_dirs:
 
       if self._get_test_module(module_dir) is None:
         loaded_module = self._load_test_module(module_dir)

From 0d299bad884bf24f00994257a059c005fced835a Mon Sep 17 00:00:00 2001
From: jhughesbiot <jonathan.hughes@buildingsiot.com>
Date: Thu, 29 Feb 2024 14:04:42 -0700
Subject: [PATCH 3/4] Add protocol display name to report

---
 framework/python/src/common/util.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/framework/python/src/common/util.py b/framework/python/src/common/util.py
index ebc3d3d70..d40571481 100644
--- a/framework/python/src/common/util.py
+++ b/framework/python/src/common/util.py
@@ -103,7 +103,8 @@ def get_module_display_name(search):
     'dns': 'DNS',
     'connection': 'Connection',
     'nmap': 'Services',
-    'tls': 'TLS'
+    'tls': 'TLS',
+    'protocol': 'Protocol'
   }
 
   for module in modules.items():

From c22369208ef179227863f7e826bfc5f97132babc Mon Sep 17 00:00:00 2001
From: jhughesbiot <jonathan.hughes@buildingsiot.com>
Date: Thu, 29 Feb 2024 14:06:49 -0700
Subject: [PATCH 4/4] Add bacnet version test

---
 modules/test/protocol/conf/module_config.json |  6 +++
 .../protocol/python/src/protocol_bacnet.py    | 41 ++++++++++++++-----
 .../protocol/python/src/protocol_module.py    | 24 +++++++++++
 3 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/modules/test/protocol/conf/module_config.json b/modules/test/protocol/conf/module_config.json
index a51a3603b..486751fb1 100644
--- a/modules/test/protocol/conf/module_config.json
+++ b/modules/test/protocol/conf/module_config.json
@@ -19,6 +19,12 @@
         "expected_behavior": "BACnet traffic can be seen on the network and packets are valid and not malformed",
         "required_result": "Recommended"
       },
+      {
+        "name": "protocol.bacnet.version",
+        "test_description": "Can valid BACnet traffic be seen",
+        "expected_behavior": "BACnet traffic can be seen on the network and packets are valid and not malformed",
+        "required_result": "Recommended"
+      },
       {
         "name": "protocol.valid_modbus",
         "test_description": "Can valid Modbus traffic be seen",
diff --git a/modules/test/protocol/python/src/protocol_bacnet.py b/modules/test/protocol/python/src/protocol_bacnet.py
index c23061c51..8560ba00f 100644
--- a/modules/test/protocol/python/src/protocol_bacnet.py
+++ b/modules/test/protocol/python/src/protocol_bacnet.py
@@ -15,15 +15,17 @@
 
 import BAC0
 import logging
+from BAC0.core.io.IOExceptions import UnknownPropertyError, ReadPropertyException, NoResponseFromController, DeviceNotConnected
 
 LOGGER = None
 BAC0_LOG = '/root/.BAC0/BAC0.log'
 
+
 class BACnet():
   """BACnet Test module"""
 
   def __init__(self, log):
-    # Set the log 
+    # Set the log
     global LOGGER
     LOGGER = log
 
@@ -33,20 +35,22 @@ def __init__(self, log):
                    stderr=logging.CRITICAL)
 
     self.devices = []
+    self.bacnet = None
 
   def discover(self, local_ip=None):
     LOGGER.info('Performing BACnet discovery...')
-    bacnet = BAC0.lite(local_ip)
-    LOGGER.info('Local BACnet object: ' + str(bacnet))
+    self.bacnet = BAC0.lite(local_ip)
+    LOGGER.info('Local BACnet object: ' + str(self.bacnet))
     try:
-      bacnet.discover(global_broadcast=True)
-    except Exception as e:
+      self.bacnet.discover(global_broadcast=True)
+    except Exception as e:  # pylint: disable=W0718
       LOGGER.error(e)
     LOGGER.info('BACnet discovery complete')
-    with open(BAC0_LOG,'r',encoding='utf-8') as f:
+    with open(BAC0_LOG, 'r', encoding='utf-8') as f:
       bac0_log = f.read()
     LOGGER.info('BAC0 Log:\n' + bac0_log)
-    self.devices = bacnet.devices
+
+    self.devices = self.bacnet.devices
 
   # Check if the device being tested is in the discovered devices list
   def validate_device(self, local_ip, device_ip):
@@ -57,9 +61,8 @@ def validate_device(self, local_ip, device_ip):
     if len(self.devices) > 0:
       # Load a fail result initially and pass only
       # if we can validate it's the right device responding
-      result = False, (
-        'Could not confirm discovered BACnet device is the ' +
-        'same as device being tested')
+      result = False, ('Could not confirm discovered BACnet device is the ' +
+                       'same as device being tested')
       for device in self.devices:
         address = device[2]
         LOGGER.info('Checking device: ' + str(device))
@@ -71,3 +74,21 @@ def validate_device(self, local_ip, device_ip):
     if result is not None:
       LOGGER.info(result[1])
     return result
+
+  def validate_protocol_version(self, device_ip, device_id):
+    LOGGER.info(f'Resolving protocol version for BACnet device: {device_id}')
+    try:
+      version = self.bacnet.read(
+          f'{device_ip} device {device_id} protocolVersion')
+      revision = self.bacnet.read(
+          f'{device_ip} device {device_id} protocolRevision')
+      protocol_version = f'{version}.{revision}'
+      result = True
+      result_description = (
+          f'BACnet protocol version detected: {protocol_version}')
+    except (UnknownPropertyError, ReadPropertyException,
+            NoResponseFromController, DeviceNotConnected) as e:
+      result = False
+      result_description = f'Failed to resolve protocol version {e}'
+      LOGGER.error(result_description)
+    return result, result_description
diff --git a/modules/test/protocol/python/src/protocol_module.py b/modules/test/protocol/python/src/protocol_module.py
index b3233b6ae..7cbdabb2d 100644
--- a/modules/test/protocol/python/src/protocol_module.py
+++ b/modules/test/protocol/python/src/protocol_module.py
@@ -44,6 +44,30 @@ def _protocol_valid_bacnet(self):
       result = None, 'Could not resolve test container IP for BACnet discovery'
     return result
 
+  def _protocol_bacnet_version(self):
+    """
+    Validates the BACnet version of the discovered device.
+    The `protocol_valid_bacnet` test must be enabled and successful before
+    this test can pass.
+    """
+    LOGGER.info('Running protocol.bacnet.version')
+    result_status = None
+    result_description = ''
+
+    if len(self._bacnet.devices) > 0:
+      for device in self._bacnet.devices:
+        LOGGER.info(f'Checking BACnet version for device: {device}')
+        if self._device_ipv4_addr in device[2]:
+          result_status, result_description = \
+            self._bacnet.validate_protocol_version( device[2], device[3])
+          break
+        else:
+          LOGGER.info('Device does not match expected IP address, skipping')
+    else:
+      result_description = 'No BACnet devices discovered.'
+    LOGGER.info(result_description)
+    return result_status, result_description
+
   def _protocol_valid_modbus(self, config):
     LOGGER.info('Running protocol.valid_modbus')
     # Extract basic device connection information
