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

add NavRelPosNed #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ckampfe
Copy link

@ckampfe ckampfe commented Jun 1, 2022

Hi there, this adds the UBX-NAV-RELPOSNED message type, the spec for which you can find at 3.15.14 here

I probably wouldn't merge this just yet as I haven't yet tested it (I plan to add tests). I'm opening it early to get feedback and make sure it's up to the project's standards.

Does this look like something you're interested in?

cc @thebeekeeper

@lkolbly
Copy link
Collaborator

lkolbly commented Jun 4, 2022

Hi! Yes, this definitely looks like something we're interested in. Testing is... a little ad-hoc, we have pretty good unit tests for a lot of the framework but most of the individual packets themselves are manually tested either in an application or in ublox_cli. I don't have a reference station-capable GPS device, so let me know whenever you've tested it and are comfortable with it and I can merge. Otherwise looks good to me.

This particular packet is a little tricky because it's the first packet we have which differs substantially across different versions - it's 64 bytes in F9 HPG 1.32, but it's 40 bytes in M8 (see e.g. here).

At some point we'll figure out a solution, but for now I think we can merge this as-is. It might be worth adding a comment to the struct, that it's oriented towards F9 products.

(as a side note, the CI build failed, but it looks like it's entirely my fault so you can ignore it)

@ckampfe
Copy link
Author

ckampfe commented Jun 6, 2022

Hi! Yes, this definitely looks like something we're interested in. Testing is... a little ad-hoc, we have pretty good unit tests for a lot of the framework but most of the individual packets themselves are manually tested either in an application or in ublox_cli. I don't have a reference station-capable GPS device, so let me know whenever you've tested it and are comfortable with it and I can merge. Otherwise looks good to me.

Great, thanks @lkolbly! I should be able to test things a bit over the coming days.

This particular packet is a little tricky because it's the first packet we have which differs substantially across different versions - it's 64 bytes in F9 HPG 1.32, but it's 40 bytes in M8 (see e.g. here).

Interesting, I hadn't seen the M8 side of things, I'll take a look, thanks for the tip.

@maximeborges
Copy link
Collaborator

A not-too-hard solution would be to use feature-gating here, with the Ublox version (8 vs 9 for example), or maybe better, UBX protocol version (27 for F9P vs <= 23 for M8

@lkolbly
Copy link
Collaborator

lkolbly commented Sep 25, 2022

A not-too-hard solution would be to use feature-gating here, with the Ublox version (8 vs 9 for example), or maybe better, UBX protocol version (27 for F9P vs <= 23 for M8

@maximeborges Yeah, that would get around this problem. I think we'd have to rename each packet with its version number, e.g. NavRelPosNedF9P and NavRelPosNedM8 (in case the user wanted their binary to support both).

I wrote down my current plan in #43, to make sub-modules for products like "m8" or "f9p". I feel like that strikes a reasonable balance between nothing (what we have now) and a specific number-based scheme (which I worry would make device-agnostic code substantially more complicated).

stevenlovegrove added a commit to stevenlovegrove/ublox that referenced this pull request Aug 25, 2023
gwbres pushed a commit to stevenlovegrove/ublox that referenced this pull request Sep 4, 2023
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