[fix] Fixed modem-manager configuration for OpenWrt 21 #228

- Fixed modem-manager configuration for OpenWrt 21
- Added missing signalrate option for modemmanager interface

Closes #228

Co-authored-by: Federico Capoano <f.capoano@openwisp.io>

Author: pandafy

docs/source/backends/openwrt.rst

@@ -1713,27 +1713,29 @@ that are specific to modem-manager interfaces (2G, 3G, 4G, LTE, etc).
 
 These are the ``OpenWrt`` backend NetJSON extensions for Modem Manager interfaces:
 
-+--------------+---------+-----------------+--------------------------------------------+
-| key name     | type    | default         | allowed values                             |
-+==============+=========+=================+============================================+
-| ``proto``    | string  | ``modemanager`` | ``modemanager``                            |
-+--------------+---------+-----------------+--------------------------------------------+
-| ``apn``      | string  | empty           | APN, can be left blank                     |
-+--------------+---------+-----------------+--------------------------------------------+
-| ``pin``      | string  | empty           | PIN code, can be left blank                |
-+--------------+---------+-----------------+--------------------------------------------+
-| ``device``   | string  | empty           | path to device (see note below)            |
-+--------------+---------+-----------------+--------------------------------------------+
-| ``password`` | string  | empty           | password, can be left blank                |
-+--------------+---------+-----------------+--------------------------------------------+
-| ``username`` | string  | empty           | username, can be left blank                |
-+--------------+---------+-----------------+--------------------------------------------+
-| ``metric``   | integer | ``50``          | metric, can be left blank                  |
-+--------------+---------+-----------------+--------------------------------------------+
-| ``iptype``   | string  | ``ipv4``        | One of ``ipv4``, ``ipv6``, ``ipv4v6``      |
-+--------------+---------+-----------------+--------------------------------------------+
-| ``lowpower`` | boolean | ``False``       | low power mode                             |
-+--------------+---------+-----------------+--------------------------------------------+
++----------------+---------+-----------------+--------------------------------------------+
+| key name       | type    | default         | allowed values                             |
++================+=========+=================+============================================+
+| ``proto``      | string  | ``modemanager`` | ``modemanager``                            |
++----------------+---------+-----------------+--------------------------------------------+
+| ``apn``        | string  | empty           | APN, can be left blank                     |
++----------------+---------+-----------------+--------------------------------------------+
+| ``pin``        | string  | empty           | PIN code, can be left blank                |
++----------------+---------+-----------------+--------------------------------------------+
+| ``device``     | string  | empty           | path to device (see note below)            |
++----------------+---------+-----------------+--------------------------------------------+
+| ``password``   | string  | empty           | password, can be left blank                |
++----------------+---------+-----------------+--------------------------------------------+
+| ``username``   | string  | empty           | username, can be left blank                |
++----------------+---------+-----------------+--------------------------------------------+
+| ``metric``     | integer | ``50``          | metric, can be left blank                  |
++----------------+---------+-----------------+--------------------------------------------+
+| ``iptype``     | string  | ``ipv4``        | One of ``ipv4``, ``ipv6``, ``ipv4v6``      |
++----------------+---------+-----------------+--------------------------------------------+
+| ``lowpower``   | boolean | ``False``       | low power mode                             |
++----------------+---------+-----------------+--------------------------------------------+
+| ``signalrate`` | integer |                 | singal refresh rate in seconds             |
++----------------+---------+-----------------+--------------------------------------------+
 
 .. note::
     ``device`` is a required property but can be left empty so that
@@ -1761,7 +1763,8 @@ The following *configuration dictionary*:
                 "metric": 50,
                 "lowpower": False,
                 "name": "modem0",
-                "mtu": 1500
+                "mtu": 1500,
+                "signalrate": 5,
             }
         ]
     }
@@ -1770,16 +1773,19 @@ Will be rendered as follows::
 
     package network
 
+    config device 'device_modem0'
+            option mtu '1500'
+            option name 'modem0'
+
     config interface 'modem0'
             option apn 'apn.operator.com'
             option device '/sys/devices/platform/ahb/1b000000.usb/usb1/1-1'
-            option ifname 'wwan0'
             option lowpower '0'
             option metric '50'
-            option mtu '1500'
             option password 'pwd123456'
             option pincode '1234'
             option proto 'modemmanager'
+            option signalrate '5'
             option username 'user123'
 
 Radio settings

netjsonconfig/backends/openwrt/converters/interfaces.py

@@ -32,7 +32,8 @@ class Interfaces(OpenWrtConverter):
     }
     _device_config = {}
     _custom_protocols = ['ppp']
-    _interface_dsa_types = ['loopback', 'ethernet', 'bridge']
+    _proto_dsa_conflict = ['modemmanager', 'modem-manager']
+    _interface_dsa_types = ['loopback', 'ethernet', 'bridge'] + _proto_dsa_conflict
 
     def __set_dsa_interface(self, interface):
         """
@@ -216,7 +217,8 @@ def __intermediate_device(self, interface):
             }
         )
         # Add 'device' option in related interface configuration
-        interface['device'] = device['name']
+        if not interface.get('device', None):
+            interface['device'] = device['name']
 
         if interface['type'] != 'bridge':
             # A non-bridge interface that contains L2 options.
@@ -420,20 +422,33 @@ def __netjson_interface(self, interface):
             interface = method(interface)
         return interface
 
+    def __get_device_config_for_interface(self, interface):
+        device = interface.get('device')
+        name = interface.get('name')
+        if not name and interface.get('proto') in self._proto_dsa_conflict:
+            name = interface.get('.name')
+        device_config = self._device_config.get(device, self._device_config.get(name))
+        if not device_config:
+            return device_config
+        if interface.get('proto') in self._proto_dsa_conflict:
+            del device_config['type']
+        # ifname has been renamed to device in OpenWrt 21.02
+        if device_config.get('type') == 'bridge':
+            interface['ifname'] = 'br-{}'.format(interface.pop('device'))
+        elif interface.get('proto') not in self._proto_dsa_conflict:
+            interface['ifname'] = interface.pop('device')
+        return device_config
+
     def __netjson_dsa_interface(self, interface):
         if self.__is_device_config(interface) or interface.get('bridge_21', None):
             self.__netjson_device(interface)
         else:
-            device = interface.get('device')
-            # adding device property to interface
-            if self._device_config.get(device, None):
-                device_config = self._device_config[device]
-                # ifname has been renamed to device in OpenWrt 21.02
-                if device_config.get('type') == 'bridge':
-                    interface['ifname'] = 'br-{}'.format(interface.pop('device'))
-                else:
-                    interface['ifname'] = interface.pop('device')
-                if device_config.pop('bridge_21', None):
+            device_config = self.__get_device_config_for_interface(interface)
+            if device_config:
+                if (
+                    device_config.pop('bridge_21', None)
+                    or interface.get('proto') in self._proto_dsa_conflict
+                ):
                     for option in device_config:
                         if 'name' in option:
                             continue
@@ -483,7 +498,8 @@ def __netjson_device(self, interface):
                 interface[option] = value == '1'
             except KeyError:
                 continue
-        self._device_config[interface['name']] = interface
+        name = interface.get('name')
+        self._device_config[name] = interface
 
     def __netjson_type(self, interface):
         if 'type' in interface and interface['type'] == 'bridge':

netjsonconfig/backends/openwrt/schema.py

@@ -367,6 +367,12 @@
                                 "default": False,
                                 "propertyOrder": 1.8,
                             },
+                            "signalrate": {
+                                "type": "integer",
+                                "title": "Signal refresh rate",
+                                "propertyOrder": 1.9,
+                                "description": "singal refresh rate in seconds",
+                            },
                         }
                     },
                     {"$ref": "#/definitions/base_interface_settings"},

tests/openwrt/test_modemmanager.py tests/openwrt/test_modemmanager_dsa.pytests/openwrt/test_modemmanager.py renamed to tests/openwrt/test_modemmanager_dsa.py

@@ -21,23 +21,27 @@ class TestModemManager(unittest.TestCase, _TabsMixin):
                 "iptype": "ipv4v6",
                 "lowpower": False,
                 "mtu": 1500,
+                "signalrate": 5,
             }
         ]
     }
 
     _modemmanager_interface_uci = """package network
 
+config device 'device_wwan0'
+    option mtu '1500'
+    option name 'wwan0'
+
 config interface 'wwan0'
     option apn 'apn.vodafone.com'
     option device '/sys/devices/platform/ahb/1b000000.usb/usb1/1-1'
-    option ifname 'wwan0'
     option iptype 'ipv4v6'
     option lowpower '0'
     option metric '50'
-    option mtu '1500'
     option password 'pwd123456'
     option pincode '1234'
     option proto 'modemmanager'
+    option signalrate '5'
     option username 'user123'
 """
 
@@ -49,4 +53,4 @@ def test_render_modemmanager_interface(self):
     def test_parse_modemmanager_interface(self):
         result = OpenWrt(native=self._modemmanager_interface_uci).config
         expected = self._modemmanager_interface_netjson
-        self.assertDictEqual(result, expected)
+        self.assertEqual(result, expected)

tests/openwrt/test_modemmanager_legacy.py

@@ -0,0 +1,54 @@
+import unittest
+
+from netjsonconfig import OpenWrt
+from netjsonconfig.utils import _TabsMixin
+
+
+class TestModemManager(unittest.TestCase, _TabsMixin):
+    maxDiff = None
+
+    _modemmanager_interface_netjson = {
+        "interfaces": [
+            {
+                "name": "wwan0",
+                "type": "modem-manager",
+                "apn": "apn.vodafone.com",
+                "pin": "1234",
+                "device": "/sys/devices/platform/ahb/1b000000.usb/usb1/1-1",
+                "username": "user123",
+                "password": "pwd123456",
+                "metric": 50,
+                "iptype": "ipv4v6",
+                "lowpower": False,
+                "mtu": 1500,
+                "signalrate": 5,
+            }
+        ]
+    }
+
+    _modemmanager_interface_uci = """package network
+
+config interface 'wwan0'
+    option apn 'apn.vodafone.com'
+    option device '/sys/devices/platform/ahb/1b000000.usb/usb1/1-1'
+    option ifname 'wwan0'
+    option iptype 'ipv4v6'
+    option lowpower '0'
+    option metric '50'
+    option mtu '1500'
+    option password 'pwd123456'
+    option pincode '1234'
+    option proto 'modemmanager'
+    option signalrate '5'
+    option username 'user123'
+"""
+
+    def test_render_modemmanager_interface(self):
+        result = OpenWrt(self._modemmanager_interface_netjson, dsa=False).render()
+        expected = self._tabs(self._modemmanager_interface_uci)
+        self.assertEqual(result, expected)
+
+    def test_parse_modemmanager_interface(self):
+        result = OpenWrt(native=self._modemmanager_interface_uci, dsa=False).config
+        expected = self._modemmanager_interface_netjson
+        self.assertDictEqual(result, expected)