Misc. fixes from Comma devel

common/android.py

@@ -4,6 +4,7 @@
 import re
 import struct
 import subprocess
+import random
 
 ANDROID = os.path.isfile('/EON')
 
@@ -17,9 +18,10 @@ def get_imei(slot):
   if slot not in ("0", "1"):
     raise ValueError("SIM slot must be 0 or 1")
 
-  ret = parse_service_call_string(["iphonesubinfo", "3" ,"i32", str(slot)])
+  ret = parse_service_call_string(service_call(["iphonesubinfo", "3" ,"i32", str(slot)]))
   if not ret:
-    ret = "000000000000000"
+    # allow non android to be identified differently
+    ret = "%015d" % random.randint(0, 1<<32)
   return ret
 
 def get_serial():
@@ -29,7 +31,7 @@ def get_serial():
   return ret
 
 def get_subscriber_info():
-  ret = parse_service_call_string(["iphonesubinfo", "7"])
+  ret = parse_service_call_string(service_call(["iphonesubinfo", "7"]))
   if ret is None or len(ret) < 8:
     return ""
   return ret
@@ -47,15 +49,23 @@ def reboot(reason=None):
     "i32", "1" # wait
   ])
 
-def parse_service_call_unpack(call, fmt):
-  r = parse_service_call_bytes(call)
+def service_call(call):
+  if not ANDROID:
+    return None
+
+  ret = subprocess.check_output(["service", "call", *call], encoding='utf8').strip()
+  if 'Parcel' not in ret:
+    return None
+
+  return parse_service_call_bytes(ret)
+
+def parse_service_call_unpack(r, fmt):
   try:
     return struct.unpack(fmt, r)[0]
   except Exception:
     return None
 
-def parse_service_call_string(call):
-  r = parse_service_call_bytes(call)
+def parse_service_call_string(r):
   try:
     r = r[8:] # Cut off length field
     r = r.decode('utf_16_be')
@@ -71,13 +81,7 @@ def parse_service_call_string(call):
   except Exception:
     return None
 
-def parse_service_call_bytes(call):
-  if not ANDROID:
-    return None
-  ret = subprocess.check_output(["service", "call", *call], encoding='utf8').strip()
-  if 'Parcel' not in ret:
-    return None
-
+def parse_service_call_bytes(ret):
   try:
     r = b""
     for hex_part in re.findall(r'[ (]([0-9a-f]{8})', ret):

common/apk.py

@@ -32,6 +32,7 @@ def start_frame():
 
 def set_package_permissions():
   pm_grant("ai.comma.plus.offroad", "android.permission.ACCESS_FINE_LOCATION")
+  pm_grant("ai.comma.plus.offroad", "android.permission.READ_PHONE_STATE")
   appops_set("ai.comma.plus.offroad", "SU", "allow")
   appops_set("ai.comma.plus.offroad", "WIFI_SCAN", "allow")
   appops_set("ai.comma.plus.offroad", "READ_EXTERNAL_STORAGE", "allow")

panda/board/safety.h

@@ -148,20 +148,20 @@ bool addr_safety_check(CAN_FIFOMailBox_TypeDef *to_push,
 
   if (index != -1) {
     // checksum check
-    if ((get_checksum != NULL) && (compute_checksum != NULL)) {
-      if (rx_checks[index].check_checksum) {
-        uint8_t checksum = get_checksum(to_push);
-        uint8_t checksum_comp = compute_checksum(to_push);
-        rx_checks[index].valid_checksum = checksum_comp == checksum;
-      }
+    if ((get_checksum != NULL) && (compute_checksum != NULL) && rx_checks[index].check_checksum) {
+      uint8_t checksum = get_checksum(to_push);
+      uint8_t checksum_comp = compute_checksum(to_push);
+      rx_checks[index].valid_checksum = checksum_comp == checksum;
+    } else {
+      rx_checks[index].valid_checksum = true;
     }
 
-    // counter check
-    if (get_counter != NULL) {
-      if (rx_checks[index].max_counter > 0U) {
-        uint8_t counter = get_counter(to_push);
-        update_counter(rx_checks, index, counter);
-      }
+    // counter check (max_counter == 0 means skip check)
+    if ((get_counter != NULL) && (rx_checks[index].max_counter > 0U)) {
+      uint8_t counter = get_counter(to_push);
+      update_counter(rx_checks, index, counter);
+    } else {
+      rx_checks[index].wrong_counters = 0U;
     }
   }
   return is_msg_valid(rx_checks, index);

panda/board/safety/safety_chrysler.h

@@ -20,36 +20,42 @@ uint32_t chrysler_ts_last = 0;
 struct sample_t chrysler_torque_meas;         // last few torques measured
 
 static int chrysler_rx_hook(CAN_FIFOMailBox_TypeDef *to_push) {
-  int bus = GET_BUS(to_push);
-  int addr = GET_ADDR(to_push);
 
-  // Measured eps torque
-  if (addr == 544) {
-    int torque_meas_new = ((GET_BYTE(to_push, 4) & 0x7U) << 8) + GET_BYTE(to_push, 5) - 1024U;
+  bool valid = addr_safety_check(to_push, chrysler_rx_checks, CHRYSLER_RX_CHECK_LEN,
+                                 NULL, NULL, NULL);
 
-    // update array of samples
-    update_sample(&chrysler_torque_meas, torque_meas_new);
-  }
+  if (valid) {
+    int bus = GET_BUS(to_push);
+    int addr = GET_ADDR(to_push);
+
+    // Measured eps torque
+    if (addr == 544) {
+      int torque_meas_new = ((GET_BYTE(to_push, 4) & 0x7U) << 8) + GET_BYTE(to_push, 5) - 1024U;
 
-  // enter controls on rising edge of ACC, exit controls on ACC off
-  if (addr == 0x1F4) {
-    int cruise_engaged = ((GET_BYTE(to_push, 2) & 0x38) >> 3) == 7;
-    if (cruise_engaged && !chrysler_cruise_engaged_last) {
-      controls_allowed = 1;
+      // update array of samples
+      update_sample(&chrysler_torque_meas, torque_meas_new);
     }
-    if (!cruise_engaged) {
-      controls_allowed = 0;
+
+    // enter controls on rising edge of ACC, exit controls on ACC off
+    if (addr == 0x1F4) {
+      int cruise_engaged = ((GET_BYTE(to_push, 2) & 0x38) >> 3) == 7;
+      if (cruise_engaged && !chrysler_cruise_engaged_last) {
+        controls_allowed = 1;
+      }
+      if (!cruise_engaged) {
+        controls_allowed = 0;
+      }
+      chrysler_cruise_engaged_last = cruise_engaged;
     }
-    chrysler_cruise_engaged_last = cruise_engaged;
-  }
 
-  // TODO: add gas pressed check
+    // TODO: add gas pressed check
 
-  // check if stock camera ECU is on bus 0
-  if ((safety_mode_cnt > RELAY_TRNS_TIMEOUT) && (bus == 0) && (addr == 0x292)) {
-    relay_malfunction = true;
+    // check if stock camera ECU is on bus 0
+    if ((safety_mode_cnt > RELAY_TRNS_TIMEOUT) && (bus == 0) && (addr == 0x292)) {
+      relay_malfunction = true;
+    }
   }
-  return 1;
+  return valid;
 }
 
 static int chrysler_tx_hook(CAN_FIFOMailBox_TypeDef *to_send) {

panda/board/safety/safety_gm.h

@@ -42,78 +42,84 @@ uint32_t gm_ts_last = 0;
 struct sample_t gm_torque_driver;         // last few driver torques measured
 
 static int gm_rx_hook(CAN_FIFOMailBox_TypeDef *to_push) {
-  int bus = GET_BUS(to_push);
-  int addr = GET_ADDR(to_push);
-
-  if (addr == 388) {
-    int torque_driver_new = ((GET_BYTE(to_push, 6) & 0x7) << 8) | GET_BYTE(to_push, 7);
-    torque_driver_new = to_signed(torque_driver_new, 11);
-    // update array of samples
-    update_sample(&gm_torque_driver, torque_driver_new);
-  }
 
-  // sample speed, really only care if car is moving or not
-  // rear left wheel speed
-  if (addr == 842) {
-    gm_moving = GET_BYTE(to_push, 0) | GET_BYTE(to_push, 1);
-  }
+  bool valid = addr_safety_check(to_push, gm_rx_checks, GM_RX_CHECK_LEN,
+                                 NULL, NULL, NULL);
 
-  // ACC steering wheel buttons
-  if (addr == 481) {
-    int button = (GET_BYTE(to_push, 5) & 0x70) >> 4;
-    switch (button) {
-      case 2:  // resume
-      case 3:  // set
-        controls_allowed = 1;
-        break;
-      case 6:  // cancel
-        controls_allowed = 0;
-        break;
-      default:
-        break;  // any other button is irrelevant
+  if (valid) {
+    int bus = GET_BUS(to_push);
+    int addr = GET_ADDR(to_push);
+
+    if (addr == 388) {
+      int torque_driver_new = ((GET_BYTE(to_push, 6) & 0x7) << 8) | GET_BYTE(to_push, 7);
+      torque_driver_new = to_signed(torque_driver_new, 11);
+      // update array of samples
+      update_sample(&gm_torque_driver, torque_driver_new);
     }
-  }
 
-  // exit controls on rising edge of brake press or on brake press when
-  // speed > 0
-  if (addr == 241) {
-    int brake = GET_BYTE(to_push, 1);
-    // Brake pedal's potentiometer returns near-zero reading
-    // even when pedal is not pressed
-    if (brake < 10) {
-      brake = 0;
+    // sample speed, really only care if car is moving or not
+    // rear left wheel speed
+    if (addr == 842) {
+      gm_moving = GET_BYTE(to_push, 0) | GET_BYTE(to_push, 1);
     }
-    if (brake && (!gm_brake_prev || gm_moving)) {
-       controls_allowed = 0;
+
+    // ACC steering wheel buttons
+    if (addr == 481) {
+      int button = (GET_BYTE(to_push, 5) & 0x70) >> 4;
+      switch (button) {
+        case 2:  // resume
+        case 3:  // set
+          controls_allowed = 1;
+          break;
+        case 6:  // cancel
+          controls_allowed = 0;
+          break;
+        default:
+          break;  // any other button is irrelevant
+      }
     }
-    gm_brake_prev = brake;
-  }
 
-  // exit controls on rising edge of gas press
-  if (addr == 417) {
-    int gas = GET_BYTE(to_push, 6);
-    if (gas && !gm_gas_prev) {
-      controls_allowed = 0;
+    // exit controls on rising edge of brake press or on brake press when
+    // speed > 0
+    if (addr == 241) {
+      int brake = GET_BYTE(to_push, 1);
+      // Brake pedal's potentiometer returns near-zero reading
+      // even when pedal is not pressed
+      if (brake < 10) {
+        brake = 0;
+      }
+      if (brake && (!gm_brake_prev || gm_moving)) {
+         controls_allowed = 0;
+      }
+      gm_brake_prev = brake;
     }
-    gm_gas_prev = gas;
-  }
 
-  // exit controls on regen paddle
-  if (addr == 189) {
-    bool regen = GET_BYTE(to_push, 0) & 0x20;
-    if (regen) {
-      controls_allowed = 0;
+    // exit controls on rising edge of gas press
+    if (addr == 417) {
+      int gas = GET_BYTE(to_push, 6);
+      if (gas && !gm_gas_prev) {
+        controls_allowed = 0;
+      }
+      gm_gas_prev = gas;
     }
-  }
 
-  // Check if ASCM or LKA camera are online
-  // on powertrain bus.
-  // 384 = ASCMLKASteeringCmd
-  // 715 = ASCMGasRegenCmd
-  if ((safety_mode_cnt > RELAY_TRNS_TIMEOUT) && (bus == 0) && ((addr == 384) || (addr == 715))) {
-    relay_malfunction = true;
+    // exit controls on regen paddle
+    if (addr == 189) {
+      bool regen = GET_BYTE(to_push, 0) & 0x20;
+      if (regen) {
+        controls_allowed = 0;
+      }
+    }
+
+    // Check if ASCM or LKA camera are online
+    // on powertrain bus.
+    // 384 = ASCMLKASteeringCmd
+    // 715 = ASCMGasRegenCmd
+    if ((safety_mode_cnt > RELAY_TRNS_TIMEOUT) && (bus == 0) && ((addr == 384) || (addr == 715))) {
+      relay_malfunction = true;
+    }
   }
-  return 1;
+  return valid;
 }
 
 // all commands: gas/regen, friction brake and steering

panda/board/safety/safety_hyundai.h

@@ -21,35 +21,41 @@ uint32_t hyundai_ts_last = 0;
 struct sample_t hyundai_torque_driver;         // last few driver torques measured
 
 static int hyundai_rx_hook(CAN_FIFOMailBox_TypeDef *to_push) {
-  int bus = GET_BUS(to_push);
-  int addr = GET_ADDR(to_push);
 
-  if (addr == 897) {
-    int torque_driver_new = ((GET_BYTES_04(to_push) >> 11) & 0xfff) - 2048;
-    // update array of samples
-    update_sample(&hyundai_torque_driver, torque_driver_new);
-  }
+  bool valid = addr_safety_check(to_push, hyundai_rx_checks, HYUNDAI_RX_CHECK_LEN,
+                                 NULL, NULL, NULL);
+
+  if (valid) {
+    int bus = GET_BUS(to_push);
+    int addr = GET_ADDR(to_push);
 
-  // enter controls on rising edge of ACC, exit controls on ACC off
-  if (addr == 1057) {
-    // 2 bits: 13-14
-    int cruise_engaged = (GET_BYTES_04(to_push) >> 13) & 0x3;
-    if (cruise_engaged && !hyundai_cruise_engaged_last) {
-      controls_allowed = 1;
+    if (addr == 897) {
+      int torque_driver_new = ((GET_BYTES_04(to_push) >> 11) & 0xfff) - 2048;
+      // update array of samples
+      update_sample(&hyundai_torque_driver, torque_driver_new);
     }
-    if (!cruise_engaged) {
-      controls_allowed = 0;
+
+    // enter controls on rising edge of ACC, exit controls on ACC off
+    if (addr == 1057) {
+      // 2 bits: 13-14
+      int cruise_engaged = (GET_BYTES_04(to_push) >> 13) & 0x3;
+      if (cruise_engaged && !hyundai_cruise_engaged_last) {
+        controls_allowed = 1;
+      }
+      if (!cruise_engaged) {
+        controls_allowed = 0;
+      }
+      hyundai_cruise_engaged_last = cruise_engaged;
     }
-    hyundai_cruise_engaged_last = cruise_engaged;
-  }
 
-  // TODO: check gas pressed
+    // TODO: check gas pressed
 
-  // check if stock camera ECU is on bus 0
-  if ((safety_mode_cnt > RELAY_TRNS_TIMEOUT) && (bus == 0) && (addr == 832)) {
-    relay_malfunction = true;
+    // check if stock camera ECU is on bus 0
+    if ((safety_mode_cnt > RELAY_TRNS_TIMEOUT) && (bus == 0) && (addr == 832)) {
+      relay_malfunction = true;
+    }
   }
-  return 1;
+  return valid;
 }
 
 static int hyundai_tx_hook(CAN_FIFOMailBox_TypeDef *to_send) {