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

Make Ublox code more readable #4727

Merged
merged 4 commits into from
Sep 22, 2024
Merged

Conversation

fifieldt
Copy link
Contributor

@fifieldt fifieldt commented Sep 16, 2024

Ublox GNSS chips comes in a myriad of versions and settings. Presently our configuration code treats all of Ublox as a monolith and in one long function does a lot of branching based on versions.

This patch:

  1. Separates version detection from setup code
  2. Separates setup into methods for each of: Ublox 6, Ublox 7/8/9, and Ublox10.
  3. Adds a macro for sending a Ublox message.

The objective of these changes was to make the code much shorter and more readable.

@fifieldt fifieldt marked this pull request as draft September 16, 2024 00:44
@fifieldt fifieldt requested a review from GPSFan September 16, 2024 00:44
@fifieldt
Copy link
Contributor Author

fifieldt commented Sep 16, 2024

Uploading for early sanity check -- I'm still in the middle of testing. Ready for review.

@GPSFan
Copy link
Contributor

GPSFan commented Sep 16, 2024

The u-blox code and the GPS init code in general can use some sprucing up. A while ago I did a slash/hack on the u-blox identifying code, it worked for v6/v7/M8/M10, and actually reduced the overall code size. Time flies and sheep happens, never got around to doing a PR, and it bit rotted with the many changes and new parts that are now supported.

I am not a programmer and welcome insights from folks that actually know what they are doing. I'm a hardware guy I've been doing u-blox stuff since the Neo-7 was a brand new part, and fiddling with GPS since the mid 90s.

I would like to explore the possibility of changing the probing so that there could be a:
#define GNSS_Model xxxx in variant.h
Where xxxx could be:
not defined and default to PROBE in GPS.cpp
or:
PROBE
UBLOX
ATGM336
TAU1202
L76K
L76B
L89
etc.......

That would keep the existing probing ability but allow individual platforms to pre-define a GNSS Model to reduce the code footprint as well as probe time.

What do you think?

@fifieldt
Copy link
Contributor Author

Thanks, yeah, one of my hopes for this patch was that it would also make it easier for GPS experts who are not necessarily coders to wrangle the public messages.

On the probe code, certainly room to do more :) For example there are probably some variants that have a ublox soldered on them that could save a lot of time if baudate 384000 was set in variant.h .Saving that for a different patch though!

@fifieldt fifieldt changed the title [WIP] Make Ublox code more readable Make Ublox code more readable Sep 16, 2024
@fifieldt fifieldt marked this pull request as ready for review September 16, 2024 10:36
@fifieldt
Copy link
Contributor Author

I've tested this on Ublox 6 and it works the same as before the patch, which was the aim!

@fifieldt fifieldt removed the request for review from GPSFan September 17, 2024 07:55
@thebentern
Copy link
Contributor

Looks good. Let's merge this one after we cut a 2.5.1 release

@fifieldt fifieldt force-pushed the ublox-cleanup branch 6 times, most recently from 1e7d17f to 37b7a91 Compare September 22, 2024 06:58
Ublox comes in a myriad of versions and settings. Presently our
configuration code does a lot of branching based on versions being
or not being present.

This patch adds version detection earlier in the piece and branches
on the set gnssModel instead to create separate setup methods for Ublox 6,
Ublox 7/8/9, and Ublox10.

Additionally, adds a macro to make the code much shorter and more
readable.
@thebentern thebentern merged commit 2e24d24 into meshtastic:master Sep 22, 2024
107 checks passed
@fifieldt fifieldt deleted the ublox-cleanup branch September 23, 2024 01:09
mverch67 pushed a commit that referenced this pull request Sep 23, 2024
* Simplify Ublox code

Ublox comes in a myriad of versions and settings. Presently our
configuration code does a lot of branching based on versions being
or not being present.

This patch adds version detection earlier in the piece and branches
on the set gnssModel instead to create separate setup methods for Ublox 6,
Ublox 7/8/9, and Ublox10.

Additionally, adds a macro to make the code much shorter and more
readable.

* Make trunk happy

* Make trunk happy

---------

Co-authored-by: Ben Meadors <[email protected]>
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