default port for bacnet; ensure INT for port and mask; fixing default BACnet config

tests/unit/connectors/bacnet/bacnet_test_mask_conversion.py

@@ -12,74 +12,84 @@ def setUp(self):
         self.default_application_config = {
             "objectName": "TB_gateway123",
             "host": "192.168.1.136",
-            "port": "47808",
-            "mask": "22",
+            "port": 47808,
+            "mask": 22,
             "objectIdentifier": 594,
             "vendorIdentifier": 11,
             "maxApduLengthAccepted": 1476,
             "segmentationSupported": "noSegmentation",
             "networkNumber": 1,
             "deviceDiscoveryTimeoutInSec": 5,
         }
+        self.default_device_config = {
+            "host": "192.168.1.50",
+            "mask": 24,
+            "port": 47808
+        }
 
-    def validate_mask(self, mask_to_test: str):
-        connector_config = {"application": dict(self.default_application_config)}
-        connector_config["application"]["mask"] = mask_to_test
+    def validate_config(self, connector_config):
         self.bacnet_connector._AsyncBACnetConnector__config = connector_config
-
         is_valid = self.bacnet_connector._AsyncBACnetConnector__is_valid_application_device_section()
-        normalized_app_config = self.bacnet_connector._AsyncBACnetConnector__config["application"]
+        return is_valid, self.bacnet_connector._AsyncBACnetConnector__config
+
+    def validate_mask(self, mask_to_test):
+        connector_config = {"application": dict(self.default_application_config)}
+        connector_config["application"]["mask"] = mask_to_test
+        is_valid, normalized_config = self.validate_config(connector_config)
+        normalized_app_config = normalized_config["application"]
         return is_valid, normalized_app_config
 
-    def test_accepts_cidr_prefixes_1_to_32(self):
-        for cidr_prefix in range(1, 33):
+    def test_accepts_integer_cidr_prefixes_0_to_32(self):
+        for cidr_prefix in range(0, 33):
             with self.subTest(cidr_prefix=cidr_prefix):
-                is_valid, app_config = self.validate_mask(str(cidr_prefix))
+                is_valid, app_config = self.validate_mask(cidr_prefix)
                 self.assertTrue(is_valid)
-                self.assertEqual(app_config["mask"], str(cidr_prefix))
-
-    def test_accepts_cidr_prefix_0_by_current_logic(self):
-        is_valid, app_config = self.validate_mask("0")
-        self.assertTrue(is_valid)
-        self.assertEqual(app_config["mask"], "0")
+                self.assertEqual(app_config["mask"], cidr_prefix)
 
-    def test_out_of_range_numeric_masks_fall_back_to_default_24(self):
-        for invalid_prefix in ["33", "100", "999999"]:
+    def test_out_of_range_integer_masks_fall_back_to_default_24(self):
+        for invalid_prefix in [33, 100, 999999, -1]:
             with self.subTest(invalid_prefix=invalid_prefix):
                 is_valid, app_config = self.validate_mask(invalid_prefix)
                 self.assertTrue(is_valid)
-                self.assertEqual(app_config["mask"], "24")
+                self.assertEqual(app_config["mask"], 24)
 
-    def test_non_numeric_masks_fall_back_to_default_24(self):
-        for invalid_mask in ["-1", "abc", "", "  ", "24/"]:
+    def test_non_integer_masks_fall_back_to_default_24(self):
+        for invalid_mask in ["24", "255.255.255.0", "abc", "", None]:
             with self.subTest(invalid_mask=invalid_mask):
                 is_valid, app_config = self.validate_mask(invalid_mask)
                 self.assertTrue(is_valid)
-                self.assertEqual(app_config["mask"], "24")
+                self.assertEqual(app_config["mask"], 24)
 
-    def test_accepts_valid_dotted_decimal_netmasks(self):
-        valid_netmasks = [
-            "255.255.255.0",  # /24
-            "255.255.255.128",  # /25
-            "255.255.255.192",  # /26
-            "255.255.254.0",  # /23
-            "255.255.252.0",  # /22
-            "255.0.0.0",  # /8
-        ]
-        for netmask in valid_netmasks:
-            with self.subTest(netmask=netmask):
-                is_valid, app_config = self.validate_mask(netmask)
-                self.assertTrue(is_valid)
-                self.assertEqual(app_config["mask"], netmask)
+    def test_missing_application_port_defaults_to_47808(self):
+        connector_config = {"application": dict(self.default_application_config)}
+        connector_config["application"].pop("port")
+        is_valid, normalized_config = self.validate_config(connector_config)
+        self.assertTrue(is_valid)
+        self.assertEqual(normalized_config["application"]["port"], 47808)
 
-    def test_invalid_dotted_decimal_netmasks_fall_back_to_default_24(self):
-        invalid_netmasks = [
-            "255.255.255.256",
-            "255.255.255",
-        ]
-        for netmask in invalid_netmasks:
-            with self.subTest(netmask=netmask):
-                is_valid, app_config = self.validate_mask(netmask)
-                self.assertTrue(is_valid)
-                self.assertEqual(app_config["mask"], "24")
+    def test_string_application_port_fails_validation(self):
+        connector_config = {"application": dict(self.default_application_config)}
+        connector_config["application"]["port"] = "47808"
+        with self.assertLogs("bacnet mask tests", level="ERROR"):
+            is_valid, _ = self.validate_config(connector_config)
+        self.assertFalse(is_valid)
 
+    def test_missing_device_port_defaults_to_47808(self):
+        connector_config = {
+            "application": dict(self.default_application_config),
+            "devices": [dict(self.default_device_config)]
+        }
+        connector_config["devices"][0].pop("port")
+        is_valid, normalized_config = self.validate_config(connector_config)
+        self.assertTrue(is_valid)
+        self.assertEqual(normalized_config["devices"][0]["port"], 47808)
+
+    def test_string_device_port_fails_validation(self):
+        connector_config = {
+            "application": dict(self.default_application_config),
+            "devices": [dict(self.default_device_config)]
+        }
+        connector_config["devices"][0]["port"] = "47808"
+        with self.assertLogs("bacnet mask tests", level="ERROR"):
+            is_valid, _ = self.validate_config(connector_config)
+        self.assertFalse(is_valid)

thingsboard_gateway/config/bacnet.json

@@ -1,9 +1,9 @@
 {
   "application": {
     "objectName": "TB_gateway",
-    "host": "0.0.0.0",
+    "host": "192.168.2.1",
     "port": 47808,
-    "mask": "24",
+    "mask": 24,
     "objectIdentifier": 599,
     "maxApduLengthAccepted": 1476,
     "segmentationSupported": "segmentedBoth",
@@ -28,8 +28,8 @@
       },
       "altResponsesAddresses": [],
       "host": "192.168.2.110",
-      "port": "47808",
-      "mask": "24",
+      "port": 47808,
+      "mask": 24,
       "pollPeriod": 10000,
       "attributes": [
         {

thingsboard_gateway/connectors/bacnet/bacnet_connector.py

@@ -216,7 +216,7 @@ async def __rescan_devices(self):
 
     async def __start(self):
         if not self.__is_valid_application_device_section():
-            self.__log.error('Can not start connector due to invalid application section in config.')
+            self.__log.error('Can not start connector due to invalid application/devices section in config.')
             return
 
         if self.__config.get('foreignDevice', {}).get('address', ''):
@@ -239,6 +239,28 @@ async def __start(self):
                              self.indication_callback(),
                              self.__application.confirmation_handler())
 
+    def __validate_and_normalize_port(self, section: dict, section_name: str) -> bool:
+        port = section.get('port')
+        if port is None:
+            section['port'] = 47808
+            return True
+
+        if isinstance(port, bool) or not isinstance(port, int):
+            self.__log.error(
+                "Invalid '%s.port': expected integer in range [1, 65535], got %r (%s).",
+                section_name, port, type(port).__name__
+            )
+            return False
+
+        if not (1 <= port <= 65535):
+            self.__log.error(
+                "Invalid '%s.port': value must be in range [1, 65535], got %d.",
+                section_name, port
+            )
+            return False
+
+        return True
+
     def __is_valid_application_device_section(self) -> bool:
         app = self.__config.get('application')
         if not isinstance(app, dict):
@@ -255,14 +277,25 @@ def __is_valid_application_device_section(self) -> bool:
             self.__log.error("Invalid IPv4 address in application section: %s", host)
             return False
 
-        port = app.get('port')
-        if not (1 <= int(port) <= 65535):
-            self.__log.error(
-                "The port inside application section must be in range [1, 65535], but got %d.",
-                port
-            )
+        if not self.__validate_and_normalize_port(app, 'application'):
+            return False
+
+        devices = self.__config.get('devices', [])
+        if not isinstance(devices, list):
+            self.__log.error("Invalid 'devices' section in config: expected list, got %s.", type(devices).__name__)
             return False
 
+        for index, device in enumerate(devices):
+            if not isinstance(device, dict):
+                self.__log.error(
+                    "Invalid 'devices[%d]' config section: expected object, got %s.",
+                    index, type(device).__name__
+                )
+                return False
+
+            if not self.__validate_and_normalize_port(device, f'devices[{index}]'):
+                return False
+
         apdu = app.get('maxApduLengthAccepted', 1476)
         if apdu not in ALLOWED_APDU:
             self.__log.warning(
@@ -271,23 +304,21 @@ def __is_valid_application_device_section(self) -> bool:
             )
             app['maxApduLengthAccepted'] = 1476
 
-        mask = app.get('mask', "24")
+        mask = app.get('mask', 24)
         try:
-            if mask.isdigit():
-                mask_int = int(mask)
-                if not (0 <= mask_int <= 32):
-                    self.__log.warning(
-                        "The mask inside application section must be in range [0, 32], but got %s. Using default 24",
-                        mask)
-                    app['mask'] = "24"
-                else:
-                    app['mask'] = mask  # valid number string
+            if not isinstance(mask, int):
+                raise TypeError("mask must be int")
+
+            if not (0 <= mask <= 32):
+                self.__log.warning(
+                    "The mask inside application section must be in range [0, 32], but got %s. Using default 24",
+                    mask)
+                app['mask'] = 24
             else:
-                # Check if it's a valid dotted mask like 255.255.255.0
-                IPv4Network(f"{host}/{mask}", strict=False)
+                app['mask'] = mask
             return True
         except Exception as e:
-            app['mask'] = "24"
+            app['mask'] = 24
             self.__log.warning("Invalid subnet mask inside application section: %s : %s using default - 24", mask,
                                str(e))
 

thingsboard_gateway/connectors/bacnet/entities/device_object_config.py

@@ -54,8 +54,7 @@ def update_address_in_config_util(config):
             address = address.rstrip('/')
         if 'mask' in config:
             network_mask = config.pop('mask', None)
-            if network_mask:
-                address += '/' + network_mask
-        if 'port' in config:
-            address += ':' + str(config.pop('port', 47808))
+            if network_mask is not None:
+                address += '/' + str(network_mask)
+        address += ':' + str(config.pop('port', 47808))
         config['address'] = address