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

Changed the log line format to comply with bGeigieMini standard. #28

Merged
merged 1 commit into from
Jan 4, 2014

Conversation

fakufaku
Copy link
Member

@fakufaku fakufaku commented Jan 4, 2014

I recently noticed that the safecastapi database is built on the structure of a bGeigieMini log line structure.

I have modified the bGeigieNano log line format to comply to this structure in order not to introduce inconsistent data in the DB.

The difference is small. Only the two last fields of the line are affected:

bGeigieMini: HDOP,FixQuality
bGeigieNano: NBSat,HDOP

Since the bGeigieNano uses the TinyGPS library that do not provide the fix quality field, I have moved back HDOP by one position and left the FixQuality field empty.

new bGeigieNano: HDOP,

@ghost ghost assigned robouden Jan 4, 2014
@Yohanan-Weininger
Copy link

see what';s currently written on data log structure in #6 in the [https://github.com/Safecast/bGeigieNanoKit/wiki/Nano-Operation-Manual], if this change reportedly correctly? or not relevant to user operation?

seanbonner added a commit that referenced this pull request Jan 4, 2014
Changed the log line format to comply with bGeigieMini standard.
@seanbonner seanbonner merged commit dcd8769 into Safecast:master Jan 4, 2014
@fakufaku
Copy link
Member Author

fakufaku commented Jan 4, 2014

@Yohanan-Weininger I have updated the wiki with the correct line format description and a link to this issue.

@nokton
Copy link
Member

nokton commented Jan 5, 2014

Hi Robin,

Not sure this is a good idea. What I understand from your modification is that for the nano you will now write HDOP,,*CHKSUM

Problem is that in the nano the HDOP field is not providing much meaningful data (at least not for me), while NBSAT shows the number of satellites locked as per the display (e.g. 8,104*CHKSUM)

Perhaps a better way is to do HDOP,NBSAT

Am I making sense?

Pieter

On Jan 4, 2014, at 7:41 PM, fakufaku notifications@github.com wrote:

I recently noticed that the safecastapi database is built on the structure of a bGeigieMini log line structure.

I have modified the bGeigieNano log line format to comply to this structure in order not to introduce inconsistent data in the DB.

The difference is small. Only the two last fields of the line are affected:

bGeigieMini: HDOP,FixQuality
bGeigieNano: NBSat,HDOP

Since the bGeigieNano uses the TinyGPS library that do not provide the fix quality field, I have moved back HDOP by one position and left the FixQuality field empty.

new bGeigieNano: HDOP,

You can merge this Pull Request by running

git pull https://github.com/fakufaku/bGeigieNanoKit master
Or view, comment on, or merge it at:

#28

Commit Summary

Changed the log line format to comply with bGeigieMini standard.
File Changes

M bGeigieNano.ino (5)
Patch Links:

https://github.com/Safecast/bGeigieNanoKit/pull/28.patch
https://github.com/Safecast/bGeigieNanoKit/pull/28.diff

Reply to this email directly or view it on GitHub.

@fakufaku
Copy link
Member Author

fakufaku commented Jan 5, 2014

Pieter,

The problem is that the NBSAT field is inconsistent with the database structure (last field is Fix Quality, not nbsat).

In the bGeigie3, I have added an extra line to the log for status related information, battery voltage, temperature, etc (could be $BNSTS here). I have put the NBSAT field in there. Of course this information does not go into the DB, but the file is archived, so the information is not lost.

My understanding is that the NBSAT is especially useful for debugging or identifying problems with the GPS but does not really need to go in the DB.

The ultimate solution would be, of course, to have a flexible DB scheme allowing for different format across different devices. I think this has been brought up a couple of time, but I do not believe it'll happen in the near future.

@nokton
Copy link
Member

nokton commented Jan 5, 2014

Hi Robin,

The solution you applied is not going to work. The real problem is that the inconsistency already happened and we now have the data that has a different meaning based on which device was used. However, leaving the field empty or not storing it are both not a way out IMHO. Actually you will make it more inconsistent as we now will have 3 formats to decode ;)

I would suggest that for now we don't change anything, but make a Github request to rename the two fields in the database to "GPS STATUS 1" and "GPS STATUS 2". This will not require any change to the existing or future bGeigies, including the bGeigie3. We then make a document that explains how to read these two status fields based on the $deviceID so that in case anyone wants to use these, they can.

We already have one case where having these fields has become useful - we had a problem with a batch of Adafruit GPS units as these had non default factory settings. As a result the availability flag was always Void, while coordinates were OK. To reuse this data in the DB we need to run a script that can set the flag to "A" based on the number of satellites locked.

Your absolutely right about the variable format. This was actually in the original design when we started - the bGeigie was to write the format of the file in the header and the reader in the API was to use that to properly store the data. The data was to be stored together with the field label for each datapoint. We should also add this to the Github for both API and all bGeigie code libraries so we can work to that flexibility over time.

Pieter

On Jan 5, 2014, at 5:18 PM, fakufaku notifications@github.com wrote:

Pieter,

The problem is that the NBSAT field is inconsistent with the database structure (last field is Fix Quality, not nbsat).

In the bGeigie3, I have added an extra line to the log for status related information (could be $BNSTS here). I have put the NBSAT field in there. Of course this information does not go into the DB, but the file is archived, so the information is not lost.

My understanding is that the NBSAT is especially useful for debugging or identifying problems with the GPS but does not really need to go in the DB.

The ultimate solution would be, of course, to have a flexible DB scheme allowing for different format across different devices. I think this has been brought up a couple of time, but I do not believe it'll happen in the near future.


Reply to this email directly or view it on GitHub.

@fakufaku
Copy link
Member Author

fakufaku commented Jan 6, 2014

Hi Pieter,

I proposed this change as a pull request because I knew there could be different strategies and I just cared to propose one of them. It is not up to me to make the final decision :)

You are free to roll-back this change as far as I am concerned.

The flexible DB will be really needed when we start having new types of measurements (CO2, etc). So it will have to be done at some point.

Robin

@nokton
Copy link
Member

nokton commented Jan 7, 2014

Yes, the flexible DB is the way to go!

My preference is to roll this back to not create more variations in formats now and focus instead on the flexible DB design.

Pieter

On Jan 6, 2014, at 6:17 PM, fakufaku notifications@github.com wrote:

Hi Pieter,

I proposed this change as a pull request because I knew there could be different strategies and I just cared to propose one of them. It is not up to me to make the final decision :)

You are free to roll-back this change as far as I am concerned.

The flexible DB will be really needed when we start having new types of measurements (CO2, etc). So it will have to be done at some point.

Robin


Reply to this email directly or view it on GitHub.

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.

5 participants