Correct brake chatter when OP lifts off brake (commaai#240)

* Add brake decel hysteresis

* typo

* modify steady-state pump to prevent chatter when OP lifts off brake

selfdrive/car/honda/carcontroller.py

@@ -37,34 +37,25 @@ def actuator_hystereses(brake, braking, brake_steady, v_ego, car_fingerprint):
   return brake, braking, brake_steady
 
 
-def brake_pump_hysteresis(apply_brake, apply_brake_last, last_pump_ts, ts):
-  # pump_on = False
-
-  # I believe the intent here was to give the pump a break after it's been on for
-  # 20 seconds. But if you trace the code with multiple calls with apply_brake = apply_brake_last,
-  # then actually the pump will turn off after .2 seconds have elapsed, NOT after 20 seconds.
-
-  # reset pump timer if:
-  # - there is an increment in brake request
-  # - we are applying steady state brakes and we haven't been running the pump
-  #   for more than 20s (to prevent pressure bleeding)
-  #if apply_brake > apply_brake_last or (ts - last_pump_ts > 20. and apply_brake > 0):
-  #  last_pump_ts = ts
-
-  # once the pump is on, run it for at least 0.2s
-  #if ts - last_pump_ts < 0.2 and apply_brake > 0:
-  #  pump_on = True
-
-  # turn pump on if we are either in steady-state braking, or want a brake increase.
-  # If brake amount has decreased, then turn the pump off. This isn't quite right yet,
-  # but it's much closer.
-  if (apply_brake >= apply_brake_last):
-    last_pump_ts = ts
+def brake_pump_hysteresis(apply_brake, apply_brake_last, last_pump_on_state, ts):
+  # If calling for more brake, turn on the pump
+  if (apply_brake > apply_brake_last):
     pump_on = True
-  else: 
+
+  # if calling for the same brake, leave the pump alone. It was either turned on 
+  # previously while braking, or it was turned off previously when apply_brake
+  # dropped below the last value. In either case, leave it as-is.
+  # Necessary because when OP is lifting its foot off the brake, we'll come in here
+  # twice with the same brake value due to the timing.
+  if (apply_brake == apply_brake_last):
+    pump_on = last_pump_on_state
+
+  if (apply_brake < apply_brake_last):
     pump_on = False
 
-  return pump_on, last_pump_ts
+  last_pump_on_state = pump_on
+
+  return pump_on, last_pump_on_state
 
 
 def process_hud_alert(hud_alert):
@@ -95,7 +86,7 @@ def __init__(self, dbc_name):
     self.brake_steady = 0.
     self.brake_last = 0.
     self.apply_brake_last = 0
-    self.last_pump_ts = 0.
+    self.last_pump_on_state = False
     self.packer = CANPacker(dbc_name)
     self.new_radar_config = False
     self.prev_lead_distance = 0.0
@@ -190,7 +181,7 @@ def update(self, enabled, CS, frame, actuators, \
       if (frame % 2) == 0:
         idx = frame // 2
         ts = frame * DT_CTRL
-        pump_on, self.last_pump_ts = brake_pump_hysteresis(apply_brake, self.apply_brake_last, self.last_pump_ts, ts)
+        pump_on, self.last_pump_on_state = brake_pump_hysteresis(apply_brake, self.apply_brake_last, self.last_pump_on_state, ts)
         # Do NOT send the cancel command if we are using the pedal. Sending cancel causes the car firmware to
         # turn the brake pump off, and we don't want that. Stock ACC does not send the cancel cmd when it is braking.
         if CS.CP.enableGasInterceptor: