From 4c6fec9ddff2cf2accc6deaea34506e5b7451311 Mon Sep 17 00:00:00 2001 From: DevTekVE Date: Fri, 21 Jun 2024 09:14:00 +0200 Subject: [PATCH 1/3] Refactor Sunnylink status checks and improve logic in multiple modules This update streamlines how the system checks the status of Sunnylink. New methods were added to `sunnylink.py` to encapsify status checking logic, such as whether the device is connected to a network, if Sunnylink is enabled, if the device is registered and if registration is needed. This resulted in simplified code in `sunnylinkd.py` and `process_config.py` as these status checks are now standardized. Furthermore, removed repetitive code in `sunnylinkd.py` by replacing it with these new methods and modified the while loop conditions according to the new status methods. --- system/athena/sunnylinkd.py | 12 +++++----- system/manager/process_config.py | 21 ++++++++--------- system/manager/sunnylink.py | 39 +++++++++++++++++++++++++++++--- 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/system/athena/sunnylinkd.py b/system/athena/sunnylinkd.py index 529d1b2059..4719fffcee 100755 --- a/system/athena/sunnylinkd.py +++ b/system/athena/sunnylinkd.py @@ -8,7 +8,6 @@ import os import threading import time -from openpilot.common.api.sunnylink import UNREGISTERED_SUNNYLINK_DONGLE_ID from openpilot.system.athena.athenad import ws_send, jsonrpc_handler, \ recv_queue, UploadQueueCache, upload_queue, cur_upload_items, backoff, ws_manage, log_handler from jsonrpc import dispatcher @@ -19,6 +18,7 @@ from openpilot.common.api import SunnylinkApi from openpilot.common.params import Params from openpilot.common.realtime import set_core_affinity from openpilot.common.swaglog import cloudlog +from openpilot.system.manager.sunnylink import sunnylink_need_register, sunnylink_ready import cereal.messaging as messaging SUNNYLINK_ATHENA_HOST = os.getenv('SUNNYLINK_ATHENA_HOST', 'wss://ws.stg.api.sunnypilot.ai') @@ -53,7 +53,7 @@ def handle_long_poll(ws: WebSocket, exit_event: threading.Event | None) -> None: thread.start() try: while not end_event.wait(0.1): - if not params.get_bool("SunnylinkEnabled"): + if not sunnylink_ready(params): cloudlog.warning("Exiting sunnylinkd.handle_long_poll as SunnylinkEnabled is False") break @@ -190,7 +190,7 @@ def main(exit_event: threading.Event = None): except Exception: cloudlog.exception("failed to set core affinity") - while params.get_bool("SunnylinkEnabled") and not params.get("SunnylinkDongleId", encoding='utf-8') not in (None, UNREGISTERED_SUNNYLINK_DONGLE_ID): + while sunnylink_need_register(params): cloudlog.info("Waiting for sunnylink registration to complete") time.sleep(10) @@ -199,7 +199,7 @@ def main(exit_event: threading.Event = None): ws_uri = SUNNYLINK_ATHENA_HOST conn_start = None conn_retries = 0 - while (exit_event is None or not exit_event.is_set()) and params.get_bool("SunnylinkEnabled"): + while (exit_event is None or not exit_event.is_set()) and sunnylink_ready(params): try: if conn_start is None: conn_start = time.monotonic() @@ -230,8 +230,8 @@ def main(exit_event: threading.Event = None): time.sleep(backoff(conn_retries)) - if not params.get_bool("SunnylinkEnabled"): - cloudlog.debug("Reached end of sunnylinkd.main while SunnylinkEnabled is False so will wait for 60 seconds before exiting") + if not sunnylink_ready(params): + cloudlog.debug("Reached end of sunnylinkd.main while sunnylink is not ready. Waiting 60s before retrying") time.sleep(60) diff --git a/system/manager/process_config.py b/system/manager/process_config.py index 5e709eae52..8a8cce69e0 100644 --- a/system/manager/process_config.py +++ b/system/manager/process_config.py @@ -1,12 +1,12 @@ import os from cereal import car -from openpilot.common.api.sunnylink import UNREGISTERED_SUNNYLINK_DONGLE_ID from openpilot.common.params import Params from openpilot.system.hardware import PC, TICI from openpilot.selfdrive.sunnypilot import get_model_generation from openpilot.system.manager.process import PythonProcess, NativeProcess, DaemonProcess from openpilot.system.mapd_manager import MAPD_PATH, COMMON_DIR +from openpilot.system.manager.sunnylink import sunnylink_need_register, sunnylink_ready WEBCAM = os.getenv("USE_WEBCAM") is not None @@ -48,16 +48,13 @@ def model_use_nav(started, params, CP: car.CarParams) -> bool: custom_model, model_gen = get_model_generation(params) return started and custom_model and model_gen not in (0, 4) +def sunnylink_ready_shim(started, params, CP: car.CarParams) -> bool: + """Shim for sunnylink_ready to match the process manager signature.""" + return sunnylink_ready(params) -def use_sunnylink(started, params, CP: car.CarParams) -> bool: - is_sunnylink_enabled = params.get_bool("SunnylinkEnabled") - is_registered = params.get("SunnylinkDongleId", encoding='utf-8') not in (None, UNREGISTERED_SUNNYLINK_DONGLE_ID) - return is_sunnylink_enabled and is_registered - -def sunnylink_need_register(started, params, CP: car.CarParams) -> bool: - is_sunnylink_enabled = params.get_bool("SunnylinkEnabled") - is_registered = params.get("SunnylinkDongleId", encoding='utf-8') not in (None, UNREGISTERED_SUNNYLINK_DONGLE_ID) - return is_sunnylink_enabled and not is_registered +def sunnylink_need_register_shim(started, params, CP: car.CarParams) -> bool: + """Shim for sunnylink_need_register to match the process manager signature.""" + return sunnylink_need_register(params) procs = [ DaemonProcess("manage_athenad", "system.athena.manage_athenad", "AthenadPid"), @@ -117,12 +114,12 @@ procs = [ # Sunnylink <3 DaemonProcess("manage_sunnylinkd", "system.athena.manage_sunnylinkd", "SunnylinkdPid"), - PythonProcess("sunnylink_registration", "system.manager.sunnylink", sunnylink_need_register), + PythonProcess("sunnylink_registration", "system.manager.sunnylink", sunnylink_need_register_shim), ] if os.path.exists("../loggerd/sunnylink_uploader.py"): procs += [ - PythonProcess("sunnylink_uploader", "system.loggerd.sunnylink_uploader", use_sunnylink), + PythonProcess("sunnylink_uploader", "system.loggerd.sunnylink_uploader", sunnylink_ready_shim), ] if os.path.exists("./gitlab_runner.sh") and not PC: diff --git a/system/manager/sunnylink.py b/system/manager/sunnylink.py index 9c396d25ea..1a70532e82 100755 --- a/system/manager/sunnylink.py +++ b/system/manager/sunnylink.py @@ -1,12 +1,40 @@ #!/usr/bin/env python3 - -from openpilot.common.api.sunnylink import SunnylinkApi +from cereal import log +from openpilot.common.api.sunnylink import SunnylinkApi, UNREGISTERED_SUNNYLINK_DONGLE_ID from openpilot.common.params import Params +from openpilot.system.hardware import HARDWARE from openpilot.system.version import is_prebuilt import time +NetworkType = log.DeviceState.NetworkType -def main(): + +def is_network_connected() -> bool: + """Check if the device is connected to a network.""" + return HARDWARE.get_network_type() != NetworkType.none + + +def get_sunnylink_status(params=Params()) -> tuple[bool, bool]: + """Get the status of Sunnylink on the device. Returns a tuple of (is_sunnylink_enabled, is_registered).""" + is_sunnylink_enabled = params.get_bool("SunnylinkEnabled") + is_registered = params.get("SunnylinkDongleId", encoding='utf-8') not in (None, UNREGISTERED_SUNNYLINK_DONGLE_ID) + return is_sunnylink_enabled, is_registered + + +def sunnylink_ready(params=Params()) -> bool: + """Check if the device is ready to communicate with Sunnylink. That means it is enabled and registered.""" + is_sunnylink_enabled, is_registered = get_sunnylink_status(params) + return is_sunnylink_enabled and is_registered + + +def sunnylink_need_register(params=Params()) -> bool: + """Check if the device needs to be registered with Sunnylink.""" + is_sunnylink_enabled, is_registered = get_sunnylink_status(params) + return is_sunnylink_enabled and not is_registered and is_network_connected() + + +def register_sunnylink(): + """Register the device with Sunnylink if it is enabled.""" extra_args = {} if not Params().get_bool("SunnylinkEnabled"): @@ -27,5 +55,10 @@ def main(): Params().put("LastSunnylinkPingTime", str(last_ping)) +def main(): + """The main method is expected to be called by the manager when the device boots up.""" + register_sunnylink() + + if __name__ == "__main__": main() From b9bec4853ce68d29755edf67c79a68875c874931 Mon Sep 17 00:00:00 2001 From: DevTekVE Date: Fri, 21 Jun 2024 09:27:45 +0200 Subject: [PATCH 2/3] Add toggle for log upload functionality A new functionality has been added to the sunnylinkd script which allows for the toggling of log uploads. This includes the establishment of "DISALLOW_LOG_UPLOAD" as a threading event and a new method, "toggleLogUpload", that sets or clears this event based on whether uploads are enabled or not. --- system/athena/sunnylinkd.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/system/athena/sunnylinkd.py b/system/athena/sunnylinkd.py index 4719fffcee..90b2379ce4 100755 --- a/system/athena/sunnylinkd.py +++ b/system/athena/sunnylinkd.py @@ -26,6 +26,7 @@ HANDLER_THREADS = int(os.getenv('HANDLER_THREADS', "4")) LOCAL_PORT_WHITELIST = {8022} SUNNYLINK_LOG_ATTR_NAME = "user.sunny.upload" SUNNYLINK_RECONNECT_TIMEOUT_S = 70 # FYI changing this will also would require a change on sidebar.cc +DISALLOW_LOG_UPLOAD = threading.Event() params = Params() sunnylink_api = SunnylinkApi(params.get("SunnylinkDongleId", encoding='utf-8')) @@ -65,7 +66,10 @@ def handle_long_poll(ws: WebSocket, exit_event: threading.Event | None) -> None: prime_type = params.get("PrimeType", encoding='utf-8') or 0 metered = sm['deviceState'].networkMetered - if metered and int(prime_type) > 2: + if DISALLOW_LOG_UPLOAD.is_set() and not comma_prime_cellular_end_event.is_set(): + cloudlog.debug(f"sunnylinkd.handle_long_poll: DISALLOW_LOG_UPLOAD, setting comma_prime_cellular_end_event") + comma_prime_cellular_end_event.set() + elif metered and int(prime_type) > 2: cloudlog.debug(f"sunnylinkd.handle_long_poll: PrimeType({prime_type}) > 2 and networkMetered({metered})") comma_prime_cellular_end_event.set() elif comma_prime_cellular_end_event.is_set(): @@ -147,6 +151,10 @@ def sunny_log_handler(end_event: threading.Event, comma_prime_cellular_end_event comma_prime_cellular_end_event.set() +@dispatcher.add_method +def toggleLogUpload(enabled: bool): + DISALLOW_LOG_UPLOAD.clear() if enabled and DISALLOW_LOG_UPLOAD.is_set() else DISALLOW_LOG_UPLOAD.set() + @dispatcher.add_method def getParamsAllKeys() -> list[str]: keys: list[str] = [k.decode('utf-8') for k in Params().all_keys()] From 1a09978811cbed14a7c2a1e8c064dcf24ab06db7 Mon Sep 17 00:00:00 2001 From: DevTekVE Date: Fri, 21 Jun 2024 10:19:45 +0200 Subject: [PATCH 3/3] Fixing a little the conditions --- system/athena/sunnylinkd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/athena/sunnylinkd.py b/system/athena/sunnylinkd.py index 90b2379ce4..ca24f24090 100755 --- a/system/athena/sunnylinkd.py +++ b/system/athena/sunnylinkd.py @@ -72,7 +72,7 @@ def handle_long_poll(ws: WebSocket, exit_event: threading.Event | None) -> None: elif metered and int(prime_type) > 2: cloudlog.debug(f"sunnylinkd.handle_long_poll: PrimeType({prime_type}) > 2 and networkMetered({metered})") comma_prime_cellular_end_event.set() - elif comma_prime_cellular_end_event.is_set(): + elif comma_prime_cellular_end_event.is_set() and not DISALLOW_LOG_UPLOAD.is_set(): cloudlog.debug(f"sunnylinkd.handle_long_poll: comma_prime_cellular_end_event is set and not PrimeType({prime_type}) > 2 or not networkMetered({metered})") comma_prime_cellular_end_event.clear() finally: