From d7b5b8aef2d85d11564eedc760341e2de0c25aef Mon Sep 17 00:00:00 2001 From: firestarsdog <229254897+firestarsdog@users.noreply.github.com> Date: Mon, 2 Nov 2026 12:31:45 -0500 Subject: [PATCH] AGNOS Modem Fix --- openpilot/common/hardware/comma/modem.py | 22 +++---- .../common/hardware/comma/tests/test_modem.py | 63 +++++++++++++++++++ 2 files changed, 74 insertions(+), 11 deletions(-) create mode 100644 openpilot/common/hardware/comma/tests/test_modem.py diff --git a/openpilot/common/hardware/comma/modem.py b/openpilot/common/hardware/comma/modem.py index 74c4781851..af4904c4ae 100755 --- a/openpilot/common/hardware/comma/modem.py +++ b/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: ,,,,,,,... 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(): diff --git a/openpilot/common/hardware/comma/tests/test_modem.py b/openpilot/common/hardware/comma/tests/test_modem.py new file mode 100644 index 0000000000..540ed75bf5 --- /dev/null +++ b/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