Fix angle offset issue, update readme and op_params descriptions (#42)

* move dynamic gas to a separate file

* update readme

* fix lane hugging mod, it would make all offsets 0 if lane hug mod was disabled. weird behavior should be fixed

* update readme

* update readme

* shorten annoying message

* test if opedit will crash with this parameter

* make op_params more robust when checking if key is live or not and updating params from file

* make op_params more robust when checking if key is live or not and updating params from file

* test

* test

* fix

* fix bug in op_edit where if user adds parameter to default params but doesn't add live key

* make op_params more robust when checking if parameter has additional information, it's more modular

* better formatting for the displaying of the modular messages

* this might look more pronounced

* fix for parameters with no extra data

* fix

* fix

* woops lol

* test

* test

* fix opTune

* test opTune

* improve the checking of parameter info

* extra check

* this might look better

* test

* test just allowed types

* update to use message function

* fix

* fixes

* add a description to op_params describing how to use it

README.md

@@ -38,6 +38,8 @@ This is my dynamic follow from 0.5, where it changes your TR (following distance
 
 Dynamic lane speed
 -----
+*This feature is disabled until I can figure out how to improve it.*
+
 This is a new feature that reduces your cruising speed if many vehicles around you are significantly slower than you. This works with and without an openpilot-identified lead. Ex.: It will slow you down if traveling in an open lane with cars in adjacent lanes that are slower than you. Or if the lead in front of the lead is slowing down, as well as cars in other lanes far ahead. The most it will slow you down is some average of: (the set speed and the average of the surrounding cars) The more the radar points, the more weight goes to the speeds of surrounding vehicles.
 
 ~~Two PID loops to control gas and brakes independently~~
@@ -46,9 +48,9 @@ This is a new feature that reduces your cruising speed if many vehicles around y
 
 If you have a Toyota Corolla with a comma pedal, you'll love this addition. Two longitudinal PID loops are set up in `longcontrol.py` so that one is running with comma pedal tuning to control the gas, and the other is running stock non-pedal tuning for better braking control. In the car, this feels miles better than stock openpilot, and nearly as good as your stock Toyota cruise control before you pulled out your DSU! It won't accelerate up to stopped cars and brake at the last moment anymore.
 
-~~Custom wheel offset to reduce lane hugging~~
+Custom wheel offset to reduce lane hugging
 -----
-***Update**: This also may be removed, I was able to get good results live tuning camera offset. Perhaps angle offset isn't needed?*
+***Update**: The performance of this modification is iffy at best, but it definitely is more apparant than just tuning your camera offset value. Removing the immediate angle offset can have some weird oscilating effect when it's windy or on roads with camber (slant to one side). Will be left in, but disabled.*
 
 Stock openpilot doesn't seem to be able to identify your car's true angle offset. With the `LaneHugging` module you can specify a custom angle offset to be added to your desired steering angle. Simply find the angle your wheel is at when you're driving on a straight highway. By default, this is disabled, to enable you can:
 - Use the `opEdit` class in the root directory of openpilot. To use it, simply open an `ssh` shell and enter the commands below:
@@ -76,13 +78,13 @@ This is a handy tool to change your `opParams` parameters without diving into an
 cd /data/openpilot
 python op_edit.py
 ```
-A list of parameters that you can change are located [here](common/op_params.py#L29).
+A list of parameters that you can modify are located [here](common/op_params.py#L42).
 
 Parameters are stored at `/data/op_params.json`
 
 Live tuning support
 -----
-This has just been added and currently only the `camera_offset` parameter is officially supported.
+This has just been added and currently only the `camera_offset` and `lane_hug_angle_offset` parameters are supported.
 - Just start opEdit with the instructions above and pick a parameter. It will let you know if it supports live tuning, if so, updates will take affect within 5 seconds!
 - Alternatively, you can use the new opTune module to live tune quicker and easier! It stays in the parameter edit view so you can more easily experiment with values. opTune show below:
 

common/op_params.py

@@ -5,6 +5,7 @@
 import random
 from common.travis_checker import travis
 
+
 def write_params(params, params_file):
   if not travis:
     with open(params_file, "w") as f:
@@ -25,22 +26,35 @@ def read_params(params_file, default_params):
 
 class opParams:
   def __init__(self):
+    """
+      To add your own parameter to opParams in your fork, simply add a new dictionary entry with the name of your parameter and its default value to save to new users' op_params.json file.
+      The description, allowed_types, and live keys are no longer required but recommended to help users edit their parameters with opEdit and opTune correctly.
+        - The description value will be shown to users when they use opEdit or opTune to change the value of the parameter.
+        - The allowed_types key is used to restrict what kinds of values can be entered with opEdit so that users can't reasonably break the fork with unintended behavior.
+          Limiting the range of floats or integers is still recommended when `.get`ting the parameter.
+          When a None value is allowed, use `type(None)` instead of None, as opEdit checks the type against the values in the key with `isinstance()`.
+        - Finally, the live key tells both opParams and opTune that it's a live parameter that will change. Therefore, you must place the `op_params.get()` call in the update function so that it can update.
+      Here's an example of the minimum required dictionary:
+
+      self.default_params = {'camera_offset': {'default': 0.06}}
+    """
+
     self.default_params = {'camera_offset': {'default': 0.06, 'allowed_types': [float, int], 'description': 'Your camera offset to use in lane_planner.py', 'live': True},
-                           'awareness_factor': {'default': 2.0, 'allowed_types': [float, int], 'description': 'Multiplier for the awareness times', 'live': False},
-                           'lane_hug_direction': {'default': None, 'allowed_types': [type(None), str], 'description': "(NoneType, 'left', 'right'): Direction of your lane hugging, if present. None will disable this modification", 'live': False},
+                           'awareness_factor': {'default': 3.0, 'allowed_types': [float, int], 'description': 'Multiplier for the awareness times', 'live': False},
+                           'lane_hug_direction': {'default': None, 'allowed_types': [type(None), str], 'description': "(None, 'left', 'right'): Direction of your lane hugging, if present. None will disable this modification", 'live': False},
                            'lane_hug_angle_offset': {'default': 0.0, 'allowed_types': [float, int], 'description': ('This is the angle your wheel reads when driving straight at highway speeds.\n'
-                                                                                                                    'Replaces both offsets generated by the calibration learner to help fix lane hugging.\n'
+                                                                                                                    'Replaces both offsets from the calibration learner to help fix lane hugging.\n'
                                                                                                                     'Enter absolute value here, direction is determined by parameter \'lane_hug_direction\''), 'live': True},
                            'following_distance': {'default': None, 'allowed_types': [type(None), float], 'description': 'None has no effect, while setting this to a float will let you change\n'
-                                                                                                                        'the TR (0.9 to 2.7, if set dynamic follow will be disabled)', 'live': False},
+                                                                                                                        'the TR (0.9 to 2.7). If this isn\'t None, dynamic follow will be disabled as well)', 'live': False},
                            'alca_nudge_required': {'default': True, 'allowed_types': [bool], 'description': ('Whether to wait for applied torque to the wheel (nudge) before making lane changes. '
                                                                                                              'If False, lane change will occur IMMEDIATELY after signaling'), 'live': False},
                            'alca_min_speed': {'default': 25.0, 'allowed_types': [float, int], 'description': 'The minimum speed allowed for an automatic lane change (in MPH)', 'live': False},
                            'static_steer_ratio': {'default': False, 'allowed_types': [bool], 'description': 'Whether you want openpilot to use the steering ratio in interface.py, or the\n'
                                                                                                             'automatically learned steering ratio. If True, it will use the static value in interface.py', 'live': False},
                            'use_dynamic_lane_speed': {'default': True, 'allowed_types': [bool], 'description': 'Whether you want openpilot to adjust your speed based on surrounding vehicles', 'live': False},
                            'min_dynamic_lane_speed': {'default': 20.0, 'allowed_types': [float, int], 'description': 'The minimum speed to allow dynamic lane speed to operate (in MPH)', 'live': False},
-                           'upload_on_hotspot': {'default': False, 'allowed_types': [bool], 'description': 'If False, openpilot will send absolutely no data when connected to your phone\'s hotspot', 'live': False},
+                           'upload_on_hotspot': {'default': False, 'allowed_types': [bool], 'description': 'If False, openpilot will not upload driving data while connected to your phone\'s hotspot', 'live': False},
                            'reset_integral': {'default': False, 'allowed_types': [bool], 'description': 'This resets integral whenever the longitudinal PID error crosses or is zero.\nShould help it recover from overshoot quicker', 'live': False}}
 
     self.params = {}
@@ -115,19 +129,21 @@ def put(self, key, value):
     self.params.update({key: value})
     write_params(self.params, self.params_file)
 
-  def get(self, key=None, default=None):  # can specify a default value if key doesn't exist
+  def get(self, key=None, default=None, force_update=False):  # can specify a default value if key doesn't exist
+    self.update_params(key, force_update)
     if key is None:
       return self.params
-    if not travis and key in self.default_params and self.default_params[key]['live']:  # if is a live param, we want to get updates while openpilot is running
-      if time.time() - self.last_read_time >= self.read_frequency:  # make sure we aren't reading file too often
+    return self.params[key] if key in self.params else default
+
+  def update_params(self, key, force_update):
+    if force_update or (key in self.default_params and 'live' in self.default_params[key] and self.default_params[key]['live']):  # if is a live param, we want to get updates while openpilot is running
+      if not travis and time.time() - self.last_read_time >= self.read_frequency:  # make sure we aren't reading file too often
         self.params, read_status = read_params(self.params_file, self.format_default_params())
         if not read_status:
-          time.sleep(0.01)
-          self.params, read_status = read_params(self.params_file, self.format_default_params())  # if the file was being written to, retry once
+          time.sleep(1/100.)
+          self.params, _ = read_params(self.params_file, self.format_default_params())  # if the file was being written to, retry once
         self.last_read_time = time.time()
 
-    return self.params[key] if key in self.params else default
-
   def delete(self, key):
     if key in self.params:
       del self.params[key]

op_edit.py

@@ -7,17 +7,16 @@ class opEdit:  # use by running `python /data/openpilot/op_edit.py`
   def __init__(self):
     self.op_params = opParams()
     self.params = None
-    self.sleep_time = 1.0
+    self.sleep_time = 1.25
     self.run_loop()
 
   def run_loop(self):
     print('Welcome to the opParams command line editor!')
     print('Here are your parameters:\n')
     while True:
-      self.params = self.op_params.get()
-
+      self.params = self.op_params.get(force_update=True)
       values_list = [self.params[i] if len(str(self.params[i])) < 20 else '{} ... {}'.format(str(self.params[i])[:30], str(self.params[i])[-15:]) for i in self.params]
-      live = [' (live!)' if i in self.op_params.default_params and self.op_params.default_params[i]['live'] else '' for i in self.params]
+      live = [' (live!)' if i in self.op_params.default_params and 'live' in self.op_params.default_params[i] and self.op_params.default_params[i]['live'] else '' for i in self.params]
 
       to_print = ['{}. {}: {} {}'.format(idx + 1, i, values_list[idx], live[idx]) for idx, i in enumerate(self.params)]
       to_print.append('\n{}. Add new parameter!'.format(len(self.params) + 1))
@@ -45,12 +44,10 @@ def parse_choice(self, choice):
       print('Exiting opEdit!')
       return 'error', choice
     else:
-      print('\nNot an integer!\n', flush=True)
-      time.sleep(self.sleep_time)
+      self.message('Not an integer!')
       return 'retry', choice
     if choice not in range(0, len(self.params) + 2):  # three for add/delete parameter
-      print('Not in range!\n', flush=True)
-      time.sleep(self.sleep_time)
+      self.message('Not in range!')
       return 'continue', choice
 
     if choice == len(self.params):  # add new parameter
@@ -64,40 +61,45 @@ def parse_choice(self, choice):
   def change_parameter(self, choice):
     while True:
       chosen_key = list(self.params)[choice]
-      extra_info = False
+      has_description = False
+      has_allowed_types = False
       live = False
       if chosen_key in self.op_params.default_params:
-        extra_info = True
-        allowed_types = self.op_params.default_params[chosen_key]['allowed_types']
-        description = self.op_params.default_params[chosen_key]['description']
-        live = self.op_params.default_params[chosen_key]['live']
+        has_description = 'description' in self.op_params.default_params[chosen_key]
+        has_allowed_types = 'allowed_types' in self.op_params.default_params[chosen_key]
+        live = 'live' in self.op_params.default_params[chosen_key] and self.op_params.default_params[chosen_key]['live']
 
       old_value = self.params[chosen_key]
       print('Chosen parameter: {}'.format(chosen_key))
       print('Current value: {} (type: {})'.format(old_value, str(type(old_value)).split("'")[1]))
-      if extra_info:
-        print('\n- Description: {}'.format(description.replace('\n', '\n  ')))
-        print('- Allowed types: {}'.format(', '.join([str(i).split("'")[1] for i in allowed_types])))
-        if live:
-          print('- This parameter supports live tuning! Updates should take affect within 5 seconds.\n')
-          print('It\'s recommended to use the new opTune module! It\'s been streamlined to make live tuning easier and quicker.')
-          print('Just exit out of this and type:')
-          print('python op_tune.py')
-          print('In the directory /data/openpilot\n')
-        else:
-          print()
+
+      to_print = []
+      if has_description:
+        to_print.append('>>  Description: {}'.format(self.op_params.default_params[chosen_key]['description'].replace('\n', '\n     ')))
+      if has_allowed_types:
+        allowed_types = self.op_params.default_params[chosen_key]['allowed_types']
+        to_print.append('>>  Allowed types: {}'.format(', '.join([str(i).split("'")[1] for i in allowed_types])))
+      if live:
+        to_print.append('>>  This parameter supports live tuning! Updates should take affect within 5 seconds.\n')
+        to_print.append('Try out opTune! It\'s designed to help you live tune parameters quicker.')
+        to_print.append('Just exit out of this and type: \'python op_tune.py\'')
+
+      if to_print:
+        print('\n{}\n'.format('\n'.join(to_print)))
+
       print('Enter your new value:')
       new_value = input('>> ').strip()
       if new_value == '':
+        self.message('Exiting this parameter...')
         return
 
       status, new_value = self.parse_input(new_value)
 
       if not status:
         continue
 
-      if extra_info and not any([isinstance(new_value, typ) for typ in allowed_types]):
-        self.message('The type of data you entered ({}) is not allowed with this parameter!\n'.format(str(type(new_value)).split("'")[1]))
+      if has_allowed_types and not any([isinstance(new_value, typ) for typ in allowed_types]):
+        self.message('The type of data you entered ({}) is not allowed with this parameter!'.format(str(type(new_value)).split("'")[1]))
         continue
 
       print('\nOld value: {} (type: {})'.format(old_value, str(type(old_value)).split("'")[1]))
@@ -106,10 +108,9 @@ def change_parameter(self, choice):
       choice = input('[Y/n]: ').lower().strip()
       if choice == 'y':
         self.op_params.put(chosen_key, new_value)
-        print('\nSaved!\n', flush=True)
+        self.message('Saved!')
       else:
-        print('\nNot saved!\n', flush=True)
-      time.sleep(self.sleep_time)
+        self.message('Not saved!')
       return
 
   def parse_input(self, dat):
@@ -147,10 +148,9 @@ def delete_parameter(self):
       choice = input('[Y/n]: ').lower().strip()
       if choice == 'y':
         self.op_params.delete(key)
-        print('\nDeleted!\n')
+        self.message('Deleted!')
       else:
-        print('\nNot saved!\n', flush=True)
-      time.sleep(self.sleep_time)
+        self.message('Not saved!')
       return
 
   def add_parameter(self):
@@ -181,10 +181,9 @@ def add_parameter(self):
       choice = input('[Y/n]: ').lower().strip()
       if choice == 'y':
         self.op_params.put(key, value)
-        print('\nSaved!\n', flush=True)
+        self.message('Saved!')
       else:
-        print('\nNot saved!\n', flush=True)
-      time.sleep(self.sleep_time)
+        self.message('Not saved!')
       return
 
   def message(self, msg):