openpilot icon indicating copy to clipboard operation
openpilot copied to clipboard

low upload / default upload / full upload toggle for cellular

Open yuzisee opened this issue 2 years ago • 7 comments

Description

Attempts to implement a toggle on cell:

▸ full upload -- i have unlimited, upload whatever (we will grab your segments over cell, not available for full prime)
▸ prime upload -- current prime policy (default)
▸ low upload -- prime policy without qlog/qcam

Original discussion https://discord.com/channels/469524606043160576/954493346250887168/1161103504761434182

Verification

I'm completely new to the openpilot codebase, so any pointers on how I can test this would be super helpful!

I used #24742 as a reference for how we make three-choice toggles, and used #23734 as a reference for how the upload policy is implemented.

Thanks!

yuzisee avatar Nov 20 '23 04:11 yuzisee

Would like to see something along the lines of an option that when you request a file upload via connect, it doesnt have to wait for wifi. Or a "force" option in the connect uploads interface to tell comma not to wait for wifi on that particular file?

Use case:

I get to work. I want to look at some log files. I request upload of said logs via Connect. Doh! Must wait until after work to dive into my logs since no wifi and I can't go out to my car to hotspot it during work.

wired4sound avatar Nov 27 '23 16:11 wired4sound

Seems like a no brainer to have this feature but as it stands, this code is not up to par to be merged.

It's probably better to modify the dispatcher with a bool param to allow uploads over cellular when requested explicitly by the user. https://github.com/commaai/openpilot/blob/6f2af97381d030fca96ad1f66c13311f6a870bee/selfdrive/athena/athenad.py#L422

However, comma connect might complain that uploads are paused on cellular but it should still work.

jakethesnake420 avatar Nov 27 '23 16:11 jakethesnake420

this code is not up to par to be merged

Yeah, happy to make improvements!

modify the dispatcher with a bool param to allow uploads over cellular when requested explicitly by the user

Cool, added https://github.com/commaai/openpilot/pull/30500/commits/74a28599a9c767dcebf3bfef7f3d052a18a04daf just now; is it close to what you had in mind?

yuzisee avatar Nov 28 '23 02:11 yuzisee

Keep it simple, this is stock openpilot. All you need to do is use the GsmMetered param in athenad and add a toggle to stop uploader when on a metered connection. Then update the tests. Oh and try testing your code and you will see it doesn't stop q uploads

jakethesnake420 avatar Nov 28 '23 15:11 jakethesnake420

Keep it simple, this is stock openpilot. All you need to do is use the GsmMetered param in athenad and add a toggle to stop uploader when on a metered connection.

Thanks. I put a simplified version here https://github.com/commaai/openpilot/pull/30569 that doesn't implement the entire original description but is much simpler and follows the approach you suggest. It's lighterweight and easier to work with for sure 🙂

Then update the tests. Oh and try testing your code and you will see it doesn't stop q uploads

Yes! As I mentioned above https://github.com/commaai/openpilot/pull/30500#issue-2001325396 I'm completely new to the openpilot codebase, so any pointers on how I can test this would be super helpful!

yuzisee avatar Dec 01 '23 18:12 yuzisee

This PR has had no activity for 30 days. It will be automatically closed in 7 days if there is no activity.

github-actions[bot] avatar Jan 01 '24 01:01 github-actions[bot]

This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.

github-actions[bot] avatar Jan 09 '24 01:01 github-actions[bot]

This PR has had no activity for 30 days. It will be automatically closed in 7 days if there is no activity.

github-actions[bot] avatar Feb 10 '24 01:02 github-actions[bot]

This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.

github-actions[bot] avatar Feb 17 '24 01:02 github-actions[bot]