-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
low upload / default upload / full upload toggle for cellular #30500
Conversation
fe08b23
to
4813edc
Compare
…els/469524606043160576/954493346250887168/1161103504761434182 for people who are looking to reduce their cellular data usage?
1a9343e
to
ece78fc
Compare
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. |
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. openpilot/selfdrive/athena/athenad.py Line 422 in 6f2af97
However, comma connect might complain that uploads are paused on cellular but it should still work. |
"It's probably better to modify the dispatcher with a bool param to allow uploads over cellular when requested explicitly by the user."
Yeah, happy to make improvements!
Cool, added 74a2859 just now; is it close to what you had in mind? |
"It's probably better to modify the dispatcher with a bool param to allow uploads over cellular when requested explicitly by the user."
07c6600
to
74a2859
Compare
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 |
…two options, instead of three. ▸ "low upload" mode (works the same as pull request commaai#30500) ▸ "default upload" mode (works the same as pull request commaai#30500) ▸ "full upload" mode: not implemented here Without a "full upload" option, we can skip checking to see whether we are using a Comma SIM (i.e. whether the user is subscribed to full prime?)
Thanks. I put a simplified version here #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 🙂
Yes! As I mentioned above #30500 (comment) I'm completely new to the openpilot codebase, so any pointers on how I can test this would be super helpful! |
This PR has had no activity for 30 days. It will be automatically closed in 7 days if there is no activity. |
This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes. |
This PR has had no activity for 30 days. It will be automatically closed in 7 days if there is no activity. |
This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes. |
Description
Attempts to implement a toggle on cell:
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!