manager tests + make all processes exit cleanly (#19595)

* manager tests

* logcatd exits cleanly

* sigint

* boardd

* multiple dbus connections hangs for some reason

* clocksd proclogd

* network type from thermal

* fix tests

* fix android logcatd

* fix mac

* fix mac proclogd

* move on device athena tests

* build first

* build first

Co-authored-by: Comma Device <device@comma.ai>

Jenkinsfile

@@ -111,9 +111,11 @@ pipeline {
                   }
                   steps {
                     phone_steps("eon", [
-                      ["build devel", "cd release && CI_PUSH=${env.CI_PUSH} ./build_devel.sh"],
+                      ["build", "SCONS_CACHE=1 scons -j4"],
-                      ["test openpilot", "nosetests -s selfdrive/test/test_openpilot.py"],
+                      ["test athena", "nosetests -s selfdrive/athena/tests/test_athenad_old.py"],
+                      ["test manager", "python selfdrive/test/test_manager.py"],
                       ["test cpu usage", "cd selfdrive/test/ && ./test_cpu_usage.py"],
+                      ["build devel", "cd release && CI_PUSH=${env.CI_PUSH} ./build_devel.sh"],
                       ["test car interfaces", "cd selfdrive/car/tests/ && ./test_car_interfaces.py"],
                       ["test spinner build", "cd selfdrive/ui/spinner && make clean && make"],
                       ["test text window build", "cd selfdrive/ui/text && make clean && make"],

release/build_release2.sh

@@ -56,7 +56,7 @@ export PYTHONPATH="/data/openpilot:/data/openpilot/pyextra"
 SCONS_CACHE=1 scons -j3
 
 # Run tests
-nosetests -s selfdrive/test/test_openpilot.py
+python selfdrive/test/test_manager.py
 selfdrive/car/tests/test_car_interfaces.py
 
 # Cleanup

release/files_common

@@ -338,9 +338,9 @@ selfdrive/thermald/power_monitoring.py
 selfdrive/test/__init__.py
 selfdrive/test/helpers.py
 selfdrive/test/setup_device_ci.sh
-selfdrive/test/test_openpilot.py
-selfdrive/test/test_fingerprints.py
 selfdrive/test/test_cpu_usage.py
+selfdrive/test/test_fingerprints.py
+selfdrive/test/test_manager.py
 
 selfdrive/ui/SConscript
 selfdrive/ui/*.cc

selfdrive/athena/tests/test_athenad.py

@@ -16,7 +16,7 @@ from websocket._exceptions import WebSocketConnectionClosedException
 
 from selfdrive.athena import athenad
 from selfdrive.athena.athenad import dispatcher
-from selfdrive.athena.test_helpers import MockWebsocket, MockParams, MockApi, EchoSocket, with_http_server
+from selfdrive.athena.tests.helpers import MockWebsocket, MockParams, MockApi, EchoSocket, with_http_server
 from cereal import messaging
 
 class TestAthenadMethods(unittest.TestCase):

selfdrive/athena/tests/test_athenad_old.py

@@ -1,62 +1,23 @@
-# flake8: noqa
+#!/usr/bin/env python3
-import os
-os.environ['FAKEUPLOAD'] = "1"
-
-from common.params import Params
-from common.realtime import sec_since_boot
-from selfdrive.manager import manager_init, manager_prepare, start_daemon_process
-from selfdrive.test.helpers import phone_only, with_processes, set_params_enabled
 import json
+import os
 import requests
 import signal
 import subprocess
 import time
 
+os.environ['FAKEUPLOAD'] = "1"
 
-# must run first
+from common.params import Params
-@phone_only
+from common.realtime import sec_since_boot
-def test_manager_prepare():
+import selfdrive.manager as manager
-  set_params_enabled()
+from selfdrive.test.helpers import with_processes
-  manager_init()
-  manager_prepare()
 
-@phone_only
-@with_processes(['loggerd', 'logmessaged', 'tombstoned', 'proclogd', 'logcatd'])
-def test_logging():
-  print("LOGGING IS SET UP")
-  time.sleep(1.0)
 
-@phone_only
-@with_processes(['camerad', 'modeld', 'dmonitoringmodeld'])
-def test_visiond():
-  print("VISIOND IS SET UP")
-  time.sleep(5.0)
-
-@phone_only
-@with_processes(['sensord'])
-def test_sensord():
-  print("SENSORS ARE SET UP")
-  time.sleep(1.0)
-
-@phone_only
-@with_processes(['ui'])
-def test_ui():
-  print("RUNNING UI")
-  time.sleep(1.0)
-
-# will have one thing to upload if loggerd ran
-# TODO: assert it actually uploaded
-@phone_only
-@with_processes(['uploader'])
-def test_uploader():
-  print("UPLOADER")
-  time.sleep(10.0)
-
-@phone_only
 def test_athena():
   print("ATHENA")
   start = sec_since_boot()
-  start_daemon_process("manage_athenad")
+  manager.start_daemon_process("manage_athenad")
   params = Params()
   manage_athenad_pid = params.get("AthenadPid")
   assert manage_athenad_pid is not None
@@ -170,9 +131,4 @@ def test_athena():
     except (OSError, TypeError):
       pass
 
-# TODO: re-enable when jenkins test has /data/pythonpath -> /data/openpilot
+
-# @phone_only
-# @with_apks()
-# def test_apks():
-#   print("APKS")
-#   time.sleep(14.0)

selfdrive/boardd/boardd.cc

@@ -39,12 +39,16 @@
 
 Panda * panda = NULL;
 std::atomic<bool> safety_setter_thread_running(false);
-volatile sig_atomic_t do_exit = 0;
 bool spoofing_started = false;
 bool fake_send = false;
 bool connected_once = false;
 bool ignition = false;
 
+volatile sig_atomic_t do_exit = 0;
+static void set_do_exit(int sig) {
+  do_exit = 1;
+}
+
 struct tm get_time(){
   time_t rawtime;
   time(&rawtime);
@@ -278,7 +282,7 @@ void can_health_thread() {
   Params params = Params();
 
   // Broadcast empty health message when panda is not yet connected
-  while (!panda){
+  while (!do_exit && !panda) {
     MessageBuilder msg;
     auto healthData  = msg.initEvent().initHealth();
 
@@ -521,6 +525,10 @@ int main() {
   err = set_core_affinity(3);
   LOG("set affinity returns %d", err);
 
+  // setup signal handlers
+  signal(SIGINT, (sighandler_t)set_do_exit);
+  signal(SIGTERM, (sighandler_t)set_do_exit);
+
   // check the environment
   if (getenv("STARTED")) {
     spoofing_started = true;

selfdrive/clocksd/clocksd.cc

@@ -1,21 +1,30 @@
-#include <chrono>
-#include <thread>
-
 #include <stdio.h>
 #include <stdint.h>
+#include <signal.h>
 #include <unistd.h>
 #include <sys/resource.h>
 #include <sys/time.h>
 
-#include <cassert>
-#include "messaging.hpp"
-#include "common/timing.h"
-
 // Apple doesn't have timerfd
-#ifndef __APPLE__
+#ifdef __APPLE__
+#include <thread>
+#else
 #include <sys/timerfd.h>
 #endif
 
+#include <cassert>
+#include <chrono>
+
+#include "messaging.hpp"
+#include "common/timing.h"
+#include "common/util.h"
+
+volatile sig_atomic_t do_exit = 0;
+static void set_do_exit(int sig) {
+  do_exit = 1;
+}
+
+
 #ifdef QCOM
 namespace {
   int64_t arm_cntpct() {
@@ -29,6 +38,9 @@ namespace {
 int main() {
   setpriority(PRIO_PROCESS, 0, -13);
 
+  signal(SIGINT, (sighandler_t)set_do_exit);
+  signal(SIGTERM, (sighandler_t)set_do_exit);
+
   PubMaster pm({"clocks"});
 
 #ifndef __APPLE__
@@ -45,11 +57,11 @@ int main() {
   assert(err == 0);
 
   uint64_t expirations = 0;
-  while ((err = read(timerfd, &expirations, sizeof(expirations)))) {
+  while (!do_exit && (err = read(timerfd, &expirations, sizeof(expirations)))) {
     if (err < 0) break;
 #else
   // Just run at 1Hz on apple
-  while (true){
+  while (!do_exit){
     std::this_thread::sleep_for(std::chrono::seconds(1));
 #endif
 

selfdrive/logcatd/logcatd_android.cc

@@ -1,6 +1,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cassert>
+#include <csignal>
 #include <android/log.h>
 
 //#include <log/log.h>
@@ -10,9 +11,18 @@
 #include "common/timing.h"
 #include "messaging.hpp"
 
+volatile sig_atomic_t do_exit = 0;
+static void set_do_exit(int sig) {
+  do_exit = 1;
+}
+
 int main() {
   int err;
 
+  // setup signal handlers
+  signal(SIGINT, (sighandler_t)set_do_exit);
+  signal(SIGTERM, (sighandler_t)set_do_exit);
+
   struct logger_list *logger_list = android_logger_list_alloc(ANDROID_LOG_RDONLY, 0, 0);
   assert(logger_list);
   struct logger *main_logger = android_logger_open(logger_list, LOG_ID_MAIN);
@@ -27,7 +37,7 @@ int main() {
   assert(kernel_logger);
   PubMaster pm({"androidLog"});
 
-  while (1) {
+  while (!do_exit) {
     log_msg log_msg;
     err = android_logger_list_read(logger_list, &log_msg);
     if (err <= 0) {

selfdrive/logcatd/logcatd_systemd.cc

@@ -1,5 +1,6 @@
 #include <iostream>
 #include <cassert>
+#include <csignal>
 #include <string>
 #include <map>
 
@@ -9,7 +10,18 @@
 #include "common/timing.h"
 #include "messaging.hpp"
 
+volatile sig_atomic_t do_exit = 0;
+
+static void set_do_exit(int sig) {
+  do_exit = 1;
+}
+
 int main(int argc, char *argv[]) {
+
+  // setup signal handlers
+  signal(SIGINT, (sighandler_t)set_do_exit);
+  signal(SIGTERM, (sighandler_t)set_do_exit);
+
   PubMaster pm({"androidLog"});
 
   sd_journal *journal;
@@ -18,16 +30,18 @@ int main(int argc, char *argv[]) {
   assert(sd_journal_seek_tail(journal) >= 0);
 
   int r;
-  while (true) {
+  while (!do_exit) {
     r = sd_journal_next(journal);
     assert(r >= 0);
 
     // Wait for new message if we didn't receive anything
     if (r == 0){
       do {
-        r = sd_journal_wait(journal, (uint64_t)-1);
+        r = sd_journal_wait(journal, 1000 * 1000);
         assert(r >= 0);
-      } while (r == SD_JOURNAL_NOP);
+      } while (r == SD_JOURNAL_NOP && !do_exit);
+
+      if (do_exit) break;
 
       r = sd_journal_next(journal);
       assert(r >= 0);

selfdrive/loggerd/tests/loggerd_tests_common.py

@@ -74,9 +74,8 @@ class UploaderTestCase(unittest.TestCase):
     uploader.ROOT = self.root  # Monkey patch root dir
     uploader.Api = MockApi
     uploader.Params = MockParams
-    uploader.fake_upload = 1
+    uploader.fake_upload = True
-    uploader.is_on_hotspot = lambda *args: False
+    uploader.force_wifi = True
-    uploader.is_on_wifi = lambda *args: True
     self.seg_num = random.randint(1, 300)
     self.seg_format = "2019-04-18--12-52-54--{}"
     self.seg_format2 = "2019-05-18--11-22-33--{}"

selfdrive/loggerd/uploader.py

@@ -1,6 +1,4 @@
 #!/usr/bin/env python3
-import ctypes
-import inspect
 import json
 import os
 import random
@@ -10,9 +8,9 @@ import time
 import traceback
 
 from cereal import log
+import cereal.messaging as messaging
 from common.api import Api
 from common.params import Params
-from selfdrive.hardware import HARDWARE
 from selfdrive.loggerd.xattr_cache import getxattr, setxattr
 from selfdrive.loggerd.config import ROOT
 from selfdrive.swaglog import cloudlog
@@ -21,31 +19,10 @@ NetworkType = log.ThermalData.NetworkType
 UPLOAD_ATTR_NAME = 'user.upload'
 UPLOAD_ATTR_VALUE = b'1'
 
+force_wifi = os.getenv("FORCEWIFI") is not None
 fake_upload = os.getenv("FAKEUPLOAD") is not None
 
 
-def raise_on_thread(t, exctype):
-  '''Raises an exception in the threads with id tid'''
-  for ctid, tobj in threading._active.items():
-    if tobj is t:
-      tid = ctid
-      break
-  else:
-    raise Exception("Could not find thread")
-
-  if not inspect.isclass(exctype):
-    raise TypeError("Only types can be raised (not instances)")
-
-  res = ctypes.pythonapi.PyThreadState_SetAsyncExc(ctypes.c_long(tid),
-                                                   ctypes.py_object(exctype))
-  if res == 0:
-    raise ValueError("invalid thread id")
-  elif res != 1:
-    # "if it returns a number greater than one, you're in trouble,
-    # and you should call it again with exc=NULL to revert the effect"
-    ctypes.pythonapi.PyThreadState_SetAsyncExc(tid, 0)
-    raise SystemError("PyThreadState_SetAsyncExc failed")
-
 def get_directory_sort(d):
   return list(map(lambda s: s.rjust(10, '0'), d.rsplit('--', 1)))
 
@@ -68,8 +45,6 @@ def clear_locks(root):
     except OSError:
       cloudlog.exception("clear_locks failed")
 
-def is_on_wifi():
-  return HARDWARE.get_network_type() == NetworkType.wifi
 
 class Uploader():
   def __init__(self, dongle_id, root):
@@ -221,17 +196,15 @@ def uploader_fn(exit_event):
     cloudlog.info("uploader missing dongle_id")
     raise Exception("uploader can't start without dongle id")
 
+  sm = messaging.SubMaster(['thermal'])
   uploader = Uploader(dongle_id, ROOT)
 
   backoff = 0.1
-  counter = 0
-  on_wifi = False
   while not exit_event.is_set():
+    sm.update(0)
+    on_wifi = force_wifi or sm['thermal'].networkType == NetworkType.wifi
     offroad = params.get("IsOffroad") == b'1'
-    allow_raw_upload = (params.get("IsUploadRawEnabled") != b"0") and offroad
+    allow_raw_upload = params.get("IsUploadRawEnabled") != b"0"
-    if offroad and counter % 12 == 0:
-      on_wifi = is_on_wifi()
-    counter += 1
 
     d = uploader.next_file_to_upload(with_raw=allow_raw_upload and on_wifi and offroad)
     if d is None:  # Nothing to upload

selfdrive/manager.py

@@ -13,7 +13,7 @@ import time
 import traceback
 
 from multiprocessing import Process
-from typing import Dict, List
+from typing import Dict
 
 from common.basedir import BASEDIR
 from common.spinner import Spinner
@@ -192,13 +192,15 @@ def get_running():
 # due to qualcomm kernel bugs SIGKILLing camerad sometimes causes page table corruption
 unkillable_processes = ['camerad']
 
-# processes to end with SIGINT instead of SIGTERM
-interrupt_processes: List[str] = []
-
 # processes to end with SIGKILL instead of SIGTERM
-kill_processes = ['sensord']
+kill_processes = []
+if EON:
+  kill_processes += [
+    'sensord',
+  ]
 
 persistent_processes = [
+  'pandad',
   'thermald',
   'logmessaged',
   'ui',
@@ -331,22 +333,21 @@ def join_process(process, timeout):
     time.sleep(0.001)
 
 
-def kill_managed_process(name):
+def kill_managed_process(name, retry=True):
   if name not in running or name not in managed_processes:
     return
-  cloudlog.info("killing %s" % name)
+  cloudlog.info(f"killing {name}")
 
   if running[name].exitcode is None:
-    if name in interrupt_processes:
+    sig = signal.SIGKILL if name in kill_processes else signal.SIGINT
-      os.kill(running[name].pid, signal.SIGINT)
+    os.kill(running[name].pid, sig)
-    elif name in kill_processes:
-      os.kill(running[name].pid, signal.SIGKILL)
-    else:
-      running[name].terminate()
 
     join_process(running[name], 5)
 
     if running[name].exitcode is None:
+      if not retry:
+        raise Exception(f"{name} failed to die")
+
       if name in unkillable_processes:
         cloudlog.critical("unkillable process %s failed to exit! rebooting in 15 if it doesn't die" % name)
         join_process(running[name], 15)
@@ -361,8 +362,10 @@ def kill_managed_process(name):
         os.kill(running[name].pid, signal.SIGKILL)
         running[name].join()
 
-  cloudlog.info("%s is dead with %d" % (name, running[name].exitcode))
+  ret = running[name].exitcode
+  cloudlog.info(f"{name} is dead with {ret}")
   del running[name]
+  return ret
 
 
 def cleanup_all_processes(signal, frame):
@@ -445,8 +448,8 @@ def manager_thread():
     pm_apply_packages('enable')
     start_offroad()
 
-  if os.getenv("NOBOARD") is None:
+  if os.getenv("NOBOARD") is not None:
-    start_managed_process("pandad")
+    del managed_processes["pandad"]
 
   if os.getenv("BLOCK") is not None:
     for k in os.getenv("BLOCK").split(","):

selfdrive/modeld/modeld.cc

@@ -11,8 +11,8 @@
 
 #include "models/driving.h"
 #include "messaging.hpp"
-volatile sig_atomic_t do_exit = 0;
 
+volatile sig_atomic_t do_exit = 0;
 static void set_do_exit(int sig) {
   do_exit = 1;
 }

selfdrive/pandad.py

@@ -3,7 +3,8 @@
 import os
 import time
 
-from panda import BASEDIR, Panda, PandaDFU, build_st
+from panda import BASEDIR as PANDA_BASEDIR, Panda, PandaDFU, build_st
+from common.basedir import BASEDIR
 from common.gpio import gpio_init, gpio_set
 from selfdrive.hardware import TICI
 from selfdrive.hardware.tici.pins import GPIO_HUB_RST_N, GPIO_STM_BOOT0, GPIO_STM_RST_N
@@ -28,7 +29,7 @@ def set_panda_power(power=True):
 
 
 def get_firmware_fn():
-  signed_fn = os.path.join(BASEDIR, "board", "obj", "panda.bin.signed")
+  signed_fn = os.path.join(PANDA_BASEDIR, "board", "obj", "panda.bin.signed")
   if os.path.exists(signed_fn):
     cloudlog.info("Using prebuilt signed firmware")
     return signed_fn
@@ -36,7 +37,7 @@ def get_firmware_fn():
     cloudlog.info("Building panda firmware")
     fn = "obj/panda.bin"
     build_st(fn, clean=False)
-    return os.path.join(BASEDIR, "board", fn)
+    return os.path.join(PANDA_BASEDIR, "board", fn)
 
 
 def get_expected_signature(fw_fn=None):
@@ -115,7 +116,7 @@ def main():
   set_panda_power()
   update_panda()
 
-  os.chdir("boardd")
+  os.chdir(os.path.join(BASEDIR, "selfdrive/boardd"))
   os.execvp("./boardd", ["./boardd"])
 
 

selfdrive/proclogd/proclogd.cc

@@ -1,10 +1,11 @@
+#include <unistd.h>
+#include <dirent.h>
+
 #include <cstdio>
 #include <cstdlib>
 #include <climits>
 #include <cassert>
-
+#include <csignal>
-#include <unistd.h>
-#include <dirent.h>
 #include <memory>
 #include <utility>
 #include <sstream>
@@ -16,10 +17,18 @@
 #include "messaging.hpp"
 
 #include "common/timing.h"
+#include "common/util.h"
 #include "common/utilpp.h"
 
+
+volatile sig_atomic_t do_exit = 0;
+
 namespace {
 
+static void set_do_exit(int sig) {
+  do_exit = 1;
+}
+
 struct ProcCache {
   std::string name;
   std::vector<std::string> cmdline;
@@ -29,6 +38,9 @@ struct ProcCache {
 }
 
 int main() {
+  signal(SIGINT, (sighandler_t)set_do_exit);
+  signal(SIGTERM, (sighandler_t)set_do_exit);
+
   PubMaster publisher({"procLog"});
 
   double jiffy = sysconf(_SC_CLK_TCK);
@@ -36,7 +48,7 @@ int main() {
 
   std::unordered_map<pid_t, ProcCache> proc_cache;
 
-  while (1) {
+  while (!do_exit) {
 
     MessageBuilder msg;
     auto procLog = msg.initEvent().initProcLog();

selfdrive/sensord/sensors_qcom2.cc

@@ -23,10 +23,9 @@
 
 #include "sensors/light_sensor.hpp"
 
-volatile sig_atomic_t do_exit = 0;
-
 #define I2C_BUS_IMU 1
 
+volatile sig_atomic_t do_exit = 0;
 
 void set_do_exit(int sig) {
   do_exit = 1;

selfdrive/test/test_manager.py

@@ -0,0 +1,59 @@
+#!/usr/bin/env python3
+import os
+import signal
+import time
+import unittest
+
+os.environ['FAKEUPLOAD'] = "1"
+
+import selfdrive.manager as manager
+from selfdrive.hardware import EON
+
+# TODO: make eon fast
+MAX_STARTUP_TIME = 30 if EON else 15
+ALL_PROCESSES = manager.persistent_processes + manager.car_started_processes
+
+class TestManager(unittest.TestCase):
+
+  def setUp(self):
+    os.environ['PASSIVE'] = '0'
+
+  def tearDown(self):
+    manager.cleanup_all_processes(None, None)
+
+  def test_manager_prepare(self):
+    os.environ['PREPAREONLY'] = '1'
+    manager.main()
+
+  def test_startup_time(self):
+    for _ in range(10):
+      start = time.monotonic()
+      os.environ['PREPAREONLY'] = '1'
+      manager.main()
+      t = time.monotonic() - start
+      assert t < MAX_STARTUP_TIME, f"startup took {t}s, expected <{MAX_STARTUP_TIME}s"
+
+  # ensure all processes exit cleanly
+  def test_clean_exit(self):
+    manager.manager_prepare()
+    for p in ALL_PROCESSES:
+      manager.start_managed_process(p)
+    
+    time.sleep(10)
+
+    for p in reversed(ALL_PROCESSES):
+      exit_code = manager.kill_managed_process(p, retry=False)
+      if not EON and (p == 'ui'or p == 'loggerd'):
+        # TODO: make Qt UI exit gracefully and fix OMX encoder exiting
+        continue
+
+      # TODO: interrupted blocking read exits with 1 in cereal. use a more unique return code
+      exit_codes = [0, 1]
+      if p in manager.kill_processes:
+        exit_codes = [-signal.SIGKILL]
+      assert exit_code in exit_codes, f"{p} died with {exit_code}"
+
+
+
+if __name__ == "__main__":
+  unittest.main()

selfdrive/thermald/thermald.py

@@ -56,7 +56,7 @@ def read_tz(x):
     return 0
 
   try:
-    with open("/sys/devices/virtual/thermal/thermal_zone%d/temp" % x) as f:
+    with open(f"/sys/devices/virtual/thermal/thermal_zone{x}/temp") as f:
       return int(f.read())
   except FileNotFoundError:
     return 0
@@ -162,10 +162,10 @@ def set_offroad_alert_if_changed(offroad_alert: str, show_alert: bool, extra_tex
 
 
 def thermald_thread():
-  health_timeout = int(1000 * 2.5 * DT_TRML)  # 2.5x the expected health frequency
 
-  # now loop
+  pm = messaging.PubMaster(['thermal'])
-  thermal_sock = messaging.pub_sock('thermal')
+
+  health_timeout = int(1000 * 2.5 * DT_TRML)  # 2.5x the expected health frequency
   health_sock = messaging.sub_sock('health', timeout=health_timeout)
   location_sock = messaging.sub_sock('gpsLocation')
 
@@ -195,15 +195,13 @@ def thermald_thread():
   is_uno = False
 
   params = Params()
-  pm = PowerMonitoring()
+  power_monitor = PowerMonitoring()
   no_panda_cnt = 0
 
   thermal_config = get_thermal_config()
 
   while 1:
     health = messaging.recv_sock(health_sock, wait=True)
-    location = messaging.recv_sock(location_sock)
-    location = location.gpsLocation if location else None
     msg = read_thermal(thermal_config)
 
     if health is not None:
@@ -278,6 +276,7 @@ def thermald_thread():
     # If device is offroad we want to cool down before going onroad
     # since going onroad increases load and can make temps go over 107
     # We only do this if there is a relay that prevents the car from faulting
+    thermal_status = ThermalStatus.green # default to good condition
     is_offroad_for_5_min = (started_ts is None) and ((not started_seen) or (off_ts is None) or (sec_since_boot() - off_ts > 60 * 5))
     if max_cpu_temp > 107. or bat_temp >= 63. or (is_offroad_for_5_min and max_cpu_temp > 70.0):
       # onroad not allowed
@@ -294,9 +293,6 @@ def thermald_thread():
     elif max_cpu_temp > 75.0:
       # hysteresis between uploader not allowed and all good
       thermal_status = clip(thermal_status, ThermalStatus.green, ThermalStatus.yellow)
-    else:
-      # all good
-      thermal_status = ThermalStatus.green
 
     # **** starting logic ****
 
@@ -365,6 +361,7 @@ def thermald_thread():
                                                                                                    log.HealthData.HwType.greyPanda]
     set_offroad_alert_if_changed("Offroad_HardwareUnsupported", health is not None and not startup_conditions["hardware_supported"])
 
+    # Handle offroad/onroad transition
     if should_start:
       if not should_start_prev:
         params.delete("IsOffroad")
@@ -376,6 +373,7 @@ def thermald_thread():
     else:
       if startup_conditions["ignition"] and (startup_conditions != startup_conditions_prev):
         cloudlog.event("Startup blocked", startup_conditions=startup_conditions)
+
       if should_start_prev or (count == 0):
         params.put("IsOffroad", "1")
 
@@ -384,15 +382,15 @@ def thermald_thread():
         off_ts = sec_since_boot()
 
     # Offroad power monitoring
-    pm.calculate(health)
+    power_monitor.calculate(health)
-    msg.thermal.offroadPowerUsage = pm.get_power_used()
+    msg.thermal.offroadPowerUsage = power_monitor.get_power_used()
-    msg.thermal.carBatteryCapacity = max(0, pm.get_car_battery_capacity())
+    msg.thermal.carBatteryCapacity = max(0, power_monitor.get_car_battery_capacity())
 
     # Check if we need to disable charging (handled by boardd)
-    msg.thermal.chargingDisabled = pm.should_disable_charging(health, off_ts)
+    msg.thermal.chargingDisabled = power_monitor.should_disable_charging(health, off_ts)
 
     # Check if we need to shut down
-    if pm.should_shutdown(health, off_ts, started_seen, LEON):
+    if power_monitor.should_shutdown(health, off_ts, started_seen, LEON):
       cloudlog.info(f"shutting device down, offroad since {off_ts}")
       # TODO: add function for blocking cloudlog instead of sleep
       time.sleep(10)
@@ -403,7 +401,7 @@ def thermald_thread():
     msg.thermal.startedTs = int(1e9*(started_ts or 0))
 
     msg.thermal.thermalStatus = thermal_status
-    thermal_sock.send(msg.to_bytes())
+    pm.send("thermal", msg)
 
     set_offroad_alert_if_changed("Offroad_ChargeDisabled", (not usb_power))
 
@@ -412,10 +410,11 @@ def thermald_thread():
 
     # report to server once per minute
     if (count % int(60. / DT_TRML)) == 0:
+      location = messaging.recv_sock(location_sock)
       cloudlog.event("STATUS_PACKET",
                      count=count,
                      health=(health.to_dict() if health else None),
-                     location=(location.to_dict() if location else None),
+                     location=(location.gpsLocation.to_dict() if location else None),
                      thermal=msg.to_dict())
 
     count += 1