AGNOS Modem Fix

openpilot/common/hardware/comma/modem.py

@@ -234,8 +234,11 @@ class Modem:
       return "unknown"
 
   @staticmethod
-  def _has_modem_manager() -> bool:
-    return os.path.isfile("/lib/systemd/system/ModemManager.service")
+  def _system_modem_service() -> str | None:
+    for service in ("modem", "ModemManager"):
+      if os.path.isfile(f"/lib/systemd/system/{service}.service"):
+        return service
+    return None
 
   def _is_roaming_allowed(self) -> bool:
     if self.S["iccid"].startswith(WEBBING_ICCID_PREFIX):
@@ -543,9 +546,7 @@ class Modem:
     return {}
 
   def _read_cellular_dns(self) -> list[str]:
-    v = self._atv(f"AT+CGCONTRDP={DIAL_CID}", "+CGCONTRDP:")
-    if not v:
-      return []
+    v = self._atv(f"AT+CGCONTRDP={DIAL_CID}", "+CGCONTRDP:") or ""
     # +CGCONTRDP: <cid>,<bearer_id>,<apn>,<local_addr>,<gw_addr>,<dns_prim>,<dns_sec>,...
     fields = [f.strip().strip('"') for f in v.split(",")]
     dns_servers = []
@@ -583,10 +584,9 @@ class Modem:
 
   def run(self):
     logging.info("starting")
+    if service := self._system_modem_service():
+      subprocess.run(["sudo", "systemctl", "mask", "--runtime", "--now", service], capture_output=True, check=True)
     self._publish_state(state=State.INITIALIZING.value)
-    if self._has_modem_manager():
-      subprocess.run(["sudo", "systemctl", "mask", "--runtime", "ModemManager"], capture_output=True)
-      subprocess.run(["sudo", "systemctl", "stop", "ModemManager"], capture_output=True)
     self._ppp.kill()
 
     state = State.INITIALIZING
@@ -620,9 +620,9 @@ class Modem:
       os.remove(STATE_PATH)
     except FileNotFoundError:
       pass
-    if self._has_modem_manager():
-      subprocess.run(["sudo", "systemctl", "unmask", "--runtime", "ModemManager"], capture_output=True)
-      subprocess.run(["sudo", "systemctl", "start", "ModemManager"], capture_output=True)
+    if service := self._system_modem_service():
+      subprocess.run(["sudo", "systemctl", "unmask", "--runtime", service], capture_output=True)
+      subprocess.run(["sudo", "systemctl", "start", service], capture_output=True)
 
 
 def main():

openpilot/common/hardware/comma/tests/test_modem.py

@@ -0,0 +1,63 @@
+import subprocess
+import unittest
+from unittest import mock
+
+from parameterized import parameterized
+
+from openpilot.common.hardware.comma.modem import Modem
+
+
+class TestModem(unittest.TestCase):
+  @parameterized.expand([
+    (('modem',), 'modem'),
+    (('ModemManager',), 'ModemManager'),
+    (('modem', 'ModemManager'), 'modem'),
+    ((), None),
+  ])
+  def test_system_modem_handoff(self, services, owner):
+    modem = Modem()
+    modem.running = False
+    units = {f'/lib/systemd/system/{name}.service' for name in services}
+    with mock.patch('os.path.isfile', side_effect=lambda path: path in units), \
+         mock.patch('subprocess.run') as run, mock.patch('os.remove'), \
+         mock.patch.object(modem, '_publish_state'), mock.patch.object(modem, '_ppp') as ppp:
+      calls = mock.Mock()
+      calls.attach_mock(run, 'systemctl')
+      calls.attach_mock(ppp.kill, 'kill')
+
+      modem.run()
+      modem.stop()
+
+      expected = []
+      if owner:
+        expected.append(mock.call.systemctl(['sudo', 'systemctl', 'mask', '--runtime', '--now', owner],
+                                            capture_output=True, check=True))
+      expected.extend([mock.call.kill(), mock.call.kill()])
+      if owner:
+        expected.extend([
+          mock.call.systemctl(['sudo', 'systemctl', 'unmask', '--runtime', owner], capture_output=True),
+          mock.call.systemctl(['sudo', 'systemctl', 'start', owner], capture_output=True),
+        ])
+      assert calls.mock_calls == expected
+
+  def test_failed_handoff_does_not_touch_modem(self):
+    modem = Modem()
+    failure = subprocess.CalledProcessError(1, ['systemctl'])
+    with mock.patch('os.path.isfile', return_value=True), \
+         mock.patch('subprocess.run', side_effect=failure), \
+         mock.patch.object(modem, '_publish_state') as publish, mock.patch.object(modem, '_ppp') as ppp:
+      with self.assertRaises(subprocess.CalledProcessError):
+        modem.run()
+      ppp.kill.assert_not_called()
+      publish.assert_not_called()
+
+  @parameterized.expand([
+    (None, ['8.8.8.8', '1.1.1.1']),
+    ('', ['8.8.8.8', '1.1.1.1']),
+    ('1,5,"apn",10.0.0.2,10.0.0.1,invalid,', ['8.8.8.8', '1.1.1.1']),
+    ('1,5,"apn",10.0.0.2,10.0.0.1,9.9.9.9,149.112.112.112', ['9.9.9.9', '149.112.112.112']),
+  ])
+  def test_cellular_dns_keeps_carrier_servers_or_uses_existing_fallback(self, response, expected):
+    modem = Modem()
+    with mock.patch.object(modem, '_atv', return_value=response):
+      assert modem._read_cellular_dns() == expected