From 91db49b9bfb5ca27068bd13378feb602e8c63f4b Mon Sep 17 00:00:00 2001
From: Adeeb Shihadeh <adeebshihadeh@gmail.com>
Date: Sun, 1 Sep 2024 10:39:49 -0700
Subject: [PATCH] athena: use dongle ID on /persist/ when available (#33259)

* athena: use dongle ID on /persist/ when available

* comment

* test

* cleanup
---
 system/athena/registration.py            | 16 ++++++++++++++--
 system/athena/tests/test_registration.py | 16 ++++++++++------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/system/athena/registration.py b/system/athena/registration.py
index 97289e4199..06ecd0b9d4 100755
--- a/system/athena/registration.py
+++ b/system/athena/registration.py
@@ -16,20 +16,32 @@ from openpilot.common.swaglog import cloudlog
 
 UNREGISTERED_DONGLE_ID = "UnregisteredDevice"
 
-
 def is_registered_device() -> bool:
   dongle = Params().get("DongleId", encoding='utf-8')
   return dongle not in (None, UNREGISTERED_DONGLE_ID)
 
 
 def register(show_spinner=False) -> str | None:
+  """
+  All devices built since March 2024 come with all
+  info stored in /persist/. This is kept around
+  only for devices built before then.
+
+  With a backend update to take serial number instead
+  of dongle ID to some endpoints, this can be removed
+  entirely.
+  """
   params = Params()
 
   IMEI = params.get("IMEI", encoding='utf8')
   HardwareSerial = params.get("HardwareSerial", encoding='utf8')
   dongle_id: str | None = params.get("DongleId", encoding='utf8')
-  needs_registration = None in (IMEI, HardwareSerial, dongle_id)
+  if dongle_id is None and Path(Paths.persist_root()+"/comma/dongle_id").is_file():
+    # not all devices will have this; added early in comma 3X production (2/28/24)
+    with open(Paths.persist_root()+"/comma/dongle_id") as f:
+      dongle_id = f.read().strip()
 
+  needs_registration = None in (IMEI, HardwareSerial, dongle_id)
   pubkey = Path(Paths.persist_root()+"/comma/id_rsa.pub")
   if not pubkey.is_file():
     dongle_id = UNREGISTERED_DONGLE_ID
diff --git a/system/athena/tests/test_registration.py b/system/athena/tests/test_registration.py
index 4f663fbc0a..17ebaa472d 100644
--- a/system/athena/tests/test_registration.py
+++ b/system/athena/tests/test_registration.py
@@ -13,13 +13,13 @@ class TestRegistration:
   def setup_method(self):
     # clear params and setup key paths
     self.params = Params()
-    self.params.clear_all()
 
     persist_dir = Path(Paths.persist_root()) / "comma"
     persist_dir.mkdir(parents=True, exist_ok=True)
 
     self.priv_key = persist_dir / "id_rsa"
     self.pub_key = persist_dir / "id_rsa.pub"
+    self.dongle_id = persist_dir / "dongle_id"
 
   def _generate_keys(self):
     self.pub_key.touch()
@@ -30,16 +30,20 @@ class TestRegistration:
       f.write(k.publickey().export_key())
 
   def test_valid_cache(self, mocker):
-    # if all params are written, return the cached dongle id
+    # if all params are written, return the cached dongle id.
+    # should work with a dongle ID on either /persist/ or normal params
     self.params.put("IMEI", "imei")
     self.params.put("HardwareSerial", "serial")
     self._generate_keys()
 
-    m = mocker.patch("openpilot.system.athena.registration.api_get", autospec=True)
     dongle = "DONGLE_ID_123"
-    self.params.put("DongleId", dongle)
-    assert register() == dongle
-    assert not m.called
+    m = mocker.patch("openpilot.system.athena.registration.api_get", autospec=True)
+    for persist, params in [(True, True), (True, False), (False, True)]:
+      self.params.put("DongleId", dongle if params else "")
+      with open(self.dongle_id, "w") as f:
+        f.write(dongle if persist else "")
+      assert register() == dongle
+      assert not m.called
 
   def test_no_keys(self, mocker):
     # missing pubkey