Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PING_INTERVAL confuses setup_autopilot on initial setup #100

Closed
cxrodgers opened this issue Jun 5, 2021 · 4 comments
Closed

PING_INTERVAL confuses setup_autopilot on initial setup #100

cxrodgers opened this issue Jun 5, 2021 · 4 comments

Comments

@cxrodgers
Copy link
Contributor

I haven't fully tracked this through yet to see what's going on, but if I am on the dev branch on a completely fresh rpi, then setup_autopilot fails to start due to PING_INTERVAL. However, on the master branch, everything is fine, so the workaround is to setup_autopilot on master.

python3 -m autopilot.setup.setup_autopilot

No existing prefs found, starting from defaults
Traceback (most recent call last):
File "/usr/lib/python3.7/runpy.py", line 193, in _run_module_as_main
"__main__", mod_spec)
File "/usr/lib/python3.7/runpy.py", line 85, in _run_code
exec(code, run_globals)
File "/home/pi/dev/autopilot/autopilot/setup/setup_autopilot.py", line 605, in <module>
setup.run()
File "/home/pi/.venv/autopilot/lib/python3.7/site-packages/npyscreen/apNPSApplication.py", line 30, in run
return npyssafewrapper.wrapper(self.__remove_argument_call_main)
File "/home/pi/.venv/autopilot/lib/python3.7/site-packages/npyscreen/npyssafewrapper.py", line 41, in wrapper
wrapper_no_fork(call_function)
File "/home/pi/.venv/autopilot/lib/python3.7/site-packages/npyscreen/npyssafewrapper.py", line 97, in wrapper_no_fork
return_code = call_function(_SCREEN)    
File "/home/pi/.venv/autopilot/lib/python3.7/site-packages/npyscreen/apNPSApplication.py", line 25, in __remove_argument_call_main
return self.main()
File "/home/pi/.venv/autopilot/lib/python3.7/site-packages/npyscreen/apNPSApplicationManaged.py", line 148, in main
self.onStart()
File "/home/pi/dev/autopilot/autopilot/setup/setup_autopilot.py", line 411, in onStart
self.forms[form_class.NAME] = self.addForm(form_class.NAME, form_class, name=form_class.DESCRIPTION)
File "/home/pi/.venv/autopilot/lib/python3.7/site-packages/npyscreen/apNPSApplicationManaged.py", line 55, in addForm
fm = FormClass( parentApp=self, *args, **keywords)
File "/home/pi/dev/autopilot/autopilot/setup/setup_autopilot.py", line 80, in __init__
super(Autopilot_Form, self).__init__(*args, **kwargs)
File "/home/pi/.venv/autopilot/lib/python3.7/site-packages/npyscreen/fmForm.py", line 70, in __init__
self.create()
File "/home/pi/dev/autopilot/autopilot/setup/setup_autopilot.py", line 362, in create
self.populate_form(PILOT_PREFS)
File "/home/pi/dev/autopilot/autopilot/setup/setup_autopilot.py", line 143, in populate_form
raise Warning("Not sure what to do with param {} with type {}".format(param_name, param['type']))
Warning: Not sure what to do with param PING_INTERVAL with type float

Probably related: once everything is properly set up, I still get these warnings upon starting autopilot:

/home/pi/dev/autopilot/autopilot/prefs.py:424: UserWarning: Returning default prefs value PING_INTERVAL : 5 (ideally this shouldnt happen and everything should be specified in prefs
  warnings.warn(f'Returning default prefs value {key} : {default_val} (ideally this shouldnt happen and everything should be specified in prefs', UserWarning)

Low-urgency bug report since the workaround works and the warning doesn't cause obvious problems

@sneakers-the-rat
Copy link
Contributor

sneakers-the-rat commented Jun 5, 2021

ok this is at once sort of funny to me and also mysterious.

i had this tragically longterm misunderstanding of how exceptions and warnings work in Python, so you'll see things like Exceptions being instantiated without being raised, and in this case raise being called on Warnings.

so what should just be a warning is being raised as if it were an exception...

but in any case it's weird to me that check is failing, because 'float' is indeed in the tuple that that if ... elif block checks: https://github.com/wehr-lab/autopilot/blob/cc866116efcf8c5cec071deb966a1702b48f3b5c/autopilot/setup/setup_autopilot.py#L112

It's fixed for me by this: 0ff28cb
but i don't have time at the moment to write a test for it. It used to be the case that we would do explicit type coercion from the strings returned by the ncurses wrapper, but instead it just uses ast.literal_eval now so that's not really necessary now, and we should just provide a textbox for a param unless it's a checkbox or etc.

the warning is intended, though it probably should be possible to silence/be clearer about what should be done. The intention is to have default values for prefs be available for cases where someone isn't using the whole system, eg. they didn't run setup_autopilot and are just trying to use one of the hardware modules, but to also let them know that there is something implicit happening that they might want to set explicitly. So the message should probably say something closer to "using default value {y} for {x}, set it explicitly in prefs with prefs.set({x}, {y}) or set 'WARN_DEFAULTS' to False to suppress this warning" (WARN_DEFAULTS isn't an implemented pref, but it probably should be)

sheesh really need to spend a day finishing up some loose ends and pulling back to main, we're drifting pretty far in unversioned territory at this point :p

@cxrodgers
Copy link
Contributor Author

That is mysterious.. maybe param['type'] is float or np.float32 or something instead of 'float'. There's something similar happening with the pigpio param where it can either be "11111110000011110000" or 11111110000011110000, which always makes me worry about integer overflow. My temptation would be to be a little more type-restrictive than type-flexible, but as long as it's not broken I figure there are bigger priorities :)

@sneakers-the-rat
Copy link
Contributor

yes i think that we need a more general notion of data typing & containers, there are a number of places where the form of data is ambiguous/brute force, (eg. Camera objects return a tuple of (timestamp, frame) but elsewhere numpy arrays are just handed around, etc.) I think this should take the form of a generalized type/declaration system that can both contain data in a predictable type as well as declare the need for it (eg. in the parameterization of a task, 'this task will return x data'). rushing out the door again but i have this idea drafted somewhere...

@sneakers-the-rat
Copy link
Contributor

closing this issue as writing tests for programmatic setup will include the remaining work here: #33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants