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

Report location provider #348

Closed
wants to merge 6 commits into from
Closed

Conversation

oliv3
Copy link
Contributor

@oliv3 oliv3 commented Aug 31, 2018

No description provided.

@oliv3 oliv3 changed the title Tell which location provider Tell which location provider was used Aug 31, 2018
@@ -91,6 +91,11 @@ public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
onCreate(db);
}

public void onDowngrade(SQLiteDatabase db, int oldVersion, int newVersion) {
db.execSQL("DROP TABLE IF EXISTS position;");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty nasty to just delete data... :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stuck with what was done for now when upgrading the schema.
Also, SQLite supports a limited subset of ALTER TABLE (https://www.sqlite.org/lang_altertable.html): there's no easy way to drop a column; and from what I know the idea is to keep the application simple.
At least this prevents the application from crashing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can you downgrade an app?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you meant to say that you have to manually download APK. I don't think very many people do that.

As for the original comment, ideally database should be empty anyway if you have internet connection. Traccar Client uploads data automatically and removes it from local database.

Copy link
Contributor Author

@oliv3 oliv3 Sep 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, regular Google Play Store users can't downgrade. But as seen @luke-jr did, so everything can happen.

Copy link
Contributor Author

@oliv3 oliv3 Sep 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for the original comment, I'm also fine with the way upgrading is done.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it by copying the apk off another phone that hadn't upgraded yet.

@oliv3 oliv3 force-pushed the locationProvider branch 3 times, most recently from 1ffa774 to 312e9cb Compare September 13, 2018 12:22
@oliv3 oliv3 changed the title Tell which location provider was used Report location provider Sep 13, 2018
Copy link

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"setting" as a name is pretty ambigous.

Re database changes: I wonder if it would make sense to just build the query string upfront and store that. Then upgrades can just send the old data as-is despite it missing new fields. (And future downgrades would safely send even the new fields!)

@oliv3
Copy link
Contributor Author

oliv3 commented Sep 13, 2018

A better name ?

@oliv3
Copy link
Contributor Author

oliv3 commented Sep 13, 2018

Database changes, I'll leave that to you. That's a harsh optimization for sure.

@oliv3
Copy link
Contributor Author

oliv3 commented Sep 13, 2018

But yes, if the job is only to replay HTTP POSTs, why not just drop Sqlite ?

@luke-jr
Copy link

luke-jr commented Sep 13, 2018

"location-source" or something like that maybe?

@oliv3
Copy link
Contributor Author

oliv3 commented Sep 13, 2018

@luke-jr no, for "setting". I'd be fine with "tuning", because that's what it does. "complicatedHeuristicName" would be too long.

@oliv3
Copy link
Contributor Author

oliv3 commented Sep 14, 2018

@tananaev ping ?

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

Successfully merging this pull request may close these issues.

3 participants