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

Added support for Beitian BN-880 module #12

Merged
merged 4 commits into from Mar 19, 2018
Merged

Conversation

tcinbis
Copy link
Contributor

@tcinbis tcinbis commented Jan 31, 2018

First of all thank you for this great library!

The Beitian BN-880 is relativly cheap module and works quite good. Unfortunately is uses slightly different NMEA sentences. But these can still be parsed with the already existing methods.

We also tested this on an ESP32 and can confirm that is works, as soon as you precompile it as a frozen module.

cintom00 added 2 commits January 31, 2018 17:19
The Beitian BN-880 appears to use slightly different NEMA sentences. By
adding these sentences and calling the methods which already exist
everything seems to work as expected. This was tested on an ESP32.

The extra exception catching was needed to prevent the program from
crashing in case a "list index out of range" exception was thrown.
All supported sentences are now in the README file and also a small part
on how to use it with an ESP32.
README.md Outdated
@@ -91,7 +99,7 @@ The timezone can be automatically adjusted for using the by setting the ```local
(3, 18, 36.0)
```

The current UTC date is stored (day,month,year). **NOTE:** The date is not currently adjusted to match the timezone set in ```local_offset```.
Copy link
Owner

Choose a reason for hiding this comment

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

Small nitpick: please avoid whitespace changes as it muddles the git history.

micropyGPS.py Outdated
@@ -497,21 +497,29 @@ def gpgsv(self):
sat_id = int(self.gps_segments[sats])
except ValueError:
return False
except Exception:
Copy link
Owner

Choose a reason for hiding this comment

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

Can you change this to IndexError since that's the exception your commit comment references? You can even just combine them: except (ValueError, IndexError):

Copy link
Owner

@inmcm inmcm left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! Just a couple small changes.

cintom00 added 2 commits February 4, 2018 10:33
The whitespaces which were removed and messed up the git history were
added again.
Instead of catching all exceptions, we just now just catch the
IndexError.
* Added missing whitspaces
@tcinbis
Copy link
Contributor Author

tcinbis commented Feb 4, 2018

@inmcm thanks for your feedback! I added the requested changes and it took me some commits until I realised that my editor was removing whitespaces for me :D

@inmcm inmcm merged commit a6eda80 into inmcm:master Mar 19, 2018
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.

None yet

2 participants