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

Remove GPS coordinates when there is no fix #233

Closed
johanstokking opened this issue Jun 25, 2021 · 4 comments · Fixed by #259
Closed

Remove GPS coordinates when there is no fix #233

johanstokking opened this issue Jun 25, 2021 · 4 comments · Fixed by #259
Assignees
Labels
bug Something isn't working
Milestone

Comments

@johanstokking
Copy link
Member

johanstokking commented Jun 25, 2021

Summary

Remove GPS coordinates when there is no fix.

cc @jamesl-dm

What do you see now?

The Oyster and Yabby codecs may return position values while these may be outdated.

Since The Things Stack Application Server from 3.13.3 is automatically publishing location solved events if the decoded payload contains coordinates, this may lead to unwanted side effects.

What do you want to see instead?

I don't want to see these position values in the fields that are used for valid values. Instead, I would put them under cached, and return a warning.

How do you propose to implement this?

Current codec:

decoded.latitudeDeg = bytes[0] + bytes[1] * 256 +
bytes[2] * 65536 + bytes[3] * 16777216;
if (decoded.latitudeDeg >= 0x80000000) // 2^31
decoded.latitudeDeg -= 0x100000000; // 2^32
decoded.latitudeDeg /= 1e7;
decoded.longitudeDeg = bytes[4] + bytes[5] * 256 +
bytes[6] * 65536 + bytes[7] * 16777216;
if (decoded.longitudeDeg >= 0x80000000) // 2^31
decoded.longitudeDeg -= 0x100000000; // 2^32
decoded.longitudeDeg /= 1e7;
decoded.inTrip = ((bytes[8] & 0x1) !== 0) ? true : false;
decoded.fixFailed = ((bytes[8] & 0x2) !== 0) ? true : false;

Suggested change is to first check whether the fix failed. If that is the case, set the position values in a cached field and return a warning.

So the result would be:

{
	"data": {
		"cached": {
			"latitudeDeg": 4.8684,
			"longitudeDeg": 52.34546,
			...
		},
		...
	},
	"warnings": ["fix failed"]
}

Can you do this yourself and submit a Pull Request?

Can review

@johanstokking johanstokking added the bug Something isn't working label Jun 25, 2021
@johanstokking johanstokking added this to the 2021 Q3 milestone Jun 25, 2021
@github-actions github-actions bot added the needs/triage We still need to triage this label Jun 25, 2021
@johanstokking johanstokking removed the needs/triage We still need to triage this label Jun 25, 2021
@Jaime-Trinidad
Copy link
Contributor

@johanstokking as I understand in cached field only will be latitude and longitude, others in this case (Oyster) batV, speed..., will be outside this label right?

Screen Shot 2021-07-14 at 16 22 36

@johanstokking
Copy link
Member Author

Battery yes, speed probably not, as that also comes from GPS.

Best is to contact the manufacturer about what is up-to-date when there is no GPS fix.

@jamesl-dm
Copy link
Contributor

That's right - latitude, longitude and speed come from the GPS.
ManDown comes from accelerometer only.

@Jaime-Trinidad
Copy link
Contributor

I contact Digitalmatter, when fixFailed = true the following uses the most recent recorded data:
Latitude
Longitude
Speed
Heading

Regardless fixFailed status the following are always considered up to date:
InTrip
manDown
vBat

So the result must be:

{
"data": {
"cached": {
"latitudeDeg": 4.8684,
"longitudeDeg": 52.34546,
"speedKmph": ...
"headingDeg": ...
},
"inTrip": false
"batV": ...
"manDown: false
},
"warnings": ["fix failed"]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants