loading progress #792
loading progress #792
Conversation
…nto subprocessing
selfdrive/manager.py
Outdated
|
|
||
| params = Params() | ||
| process_cnt = len(managed_processes) | ||
| loader_proc = subprocess.Popen(["./spinner"], stdin=subprocess.PIPE, |
There was a problem hiding this comment.
Can you abstract the spinner generation in a helper function?
There was a problem hiding this comment.
Even better, turn it into a class which which can be used using a context manager, a la tqdm:
with Spinner() as spinner:
do_stuff()
spinner.update("some text")There was a problem hiding this comment.
I think context manager sounds good. Need to think about tqdm integration.
There was a problem hiding this comment.
Didn't mean you should use tqdm, just an example of the API you could expose to our spinner.
| int err; | ||
|
|
||
| const char* spintext = NULL; | ||
| if (argc >= 2) { |
There was a problem hiding this comment.
Can we keep it backwards compatible? I'd like to initialize the text from the command line.
There was a problem hiding this comment.
I think we can. I'm not sure if it's as simple as adding that code back though. It was a little tricky to get stdin working correctly. I'd like to get ubuntu running again before I start fooling around in C.
There was a problem hiding this comment.
That would be nice. That keeps the spinner versatile and easy to call from a shells script.
|
Looks a lot cleaner this way! Added a few small comments |
|
@dekerr , we'll be happy to merge this. Are you planning on doing the requested change and solve conflicts? |
|
Looks like spinner.h is not open source in latest release. At first glance, it also looks like that makes compatibility changes in spinner.c more convoluted for this PR. |
|
Forget about the spinner. Text detailing what processes are loading, etc. would be sufficient. |
|
The spinner was refactored a little bit, so we can also use it in the NEOS installer. Unfortunately we forgot to open source the files after they were moved. I just pushed those files to devel (#812). |
|
I'll finish the PR and merge it internally! I'll probably add a progress bar too. |
|
Thanks @dekerr for figuring out the stdin stuff!
|
|
Awesome. Thanks, Willem. Looks great. |
Update interface.py
Here's a working version of #768
It's not everything I had envisioned, but unfortunately, my ubuntu hard drive died shortly after.
I still think it's useful in it's current state, so I'm putting up the PR sans progress bar for now.