-
-
Notifications
You must be signed in to change notification settings - Fork 476
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
Expanded the extended data field to contain settings that may be non-optimal for operation (Issue #1618) #1678
Conversation
Can you take a look to see if this is an acceptable direction for the PR? Still outstanding:
|
I think we clearly stated #1618 this enhancement should not be done in the |
Thanks for the PR! i agree with @ckrey here - if there's device-specific, largely static status information, let's have that in a new message type published on an |
No problem! I'll get that refactored into a different message structure. |
Any options on using true/false vs 1/0? Thoughts on 1/0 is the end consumer could sum those results. If it was non-zero then it was not optimal (using the Windows error code type mentality here...) |
Migrated to the status command and response as request. I still need to figure out the unit tests for it, but a couple questions:
... generic attributes (if any) "android": { |
by which I assume you mean HTTP request headers. If so, please keep in mind that MQTT doesn't have those, so I'd be in favor of making clear from this payload that it's Android/iOS. Nesting isn't necessary; an attribute with the OS would suffice IMO. |
like described here: #1618 (comment) |
If iOS is spelled |
Good point! I'll get that added and pushed up. |
Do you have guidance on what the proper formatting of the files should be? It's failing that portion of the build, but unsure what it is unhappy about. |
I updated the changelog + added in the MQTT unit tests. Let me know if there is anything else I'm missing. Let me know if the ktfmt failure is on my side, or part of the build process. |
Added in the status to be sent with a user trigger location to match iOS #778. This PR is ready for review now. Is the ktfmt failures due to formatting in the files I touched or a PR cannot process that? Thanks! |
Ok, figured out the ktfmt secret sauce. :) Cleanup is done and ready for submission. |
c72bfce
to
a03f0fd
Compare
…user's mobile device settings are currently in a state that negatively impacts location performance. - "wifi": 1=enabled, 0=disabled - "ps": 1=power save mode, 0=optimal power settings - "bo": 1=battery optimizations enabled, 0=optimal battery settings - "hib": 1=app can pause when unused, 0=optimal hibernation settings - "loc": location permission settings: 0 = Background location, fine precision 1 = Background location, coarse precision 2 = Foreground location, fine precision 3 = Foreground location, coarse precision 4 = Disabled
� Conflicts: � project/app/src/test/java/org/owntracks/android/model/ParserTest.kt
@wir3z I did some re-basing to put this into the next release, but don't think I can push to your branch. Could you permission me write access to |
I added you as a collaborator. Let me know if that gets you the access you need. |
Added additional return fields to the JSON packet to indicate if the user's mobile device settings are currently in a state that negatively impacts location performance.
0 = Background location, fine precision
1 = Background location, coarse precision
2 = Foreground location, fine precision
3 = Foreground location, coarse precision
4 = Disabled