-
Notifications
You must be signed in to change notification settings - Fork 194
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
V2 reduce rtcm #98
base: main
Are you sure you want to change the base?
V2 reduce rtcm #98
Conversation
@@ -677,7 +677,7 @@ int GPSDriverUBX::configureDevice(const GNSSSystemsMask &gnssSystems) | |||
cfg_valset_msg_size = initCfgValset(); | |||
cfgValset<uint8_t>(UBX_CFG_KEY_CFG_UART1OUTPROT_UBX, 1, cfg_valset_msg_size); | |||
cfgValset<uint8_t>(UBX_CFG_KEY_CFG_UART1OUTPROT_RTCM3X, 0, cfg_valset_msg_size); | |||
// heading output period 1 second | |||
// heading output period 1 second (is this once per second or once per nav cycle?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 Hz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RTCM messages are being sent at 10Hz (or 8Hz depending on GNSS config) so 8~10Hz heading solution is available. Is there any reason not to increase this to 5 or 10Hz? Would 5/10Hz be better than 4/8Hz for EKF updates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even 1 Hz is enough for ekf, it can go with the gyro alone for quite some time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree a benefit to 4/5/8/10Hz updates hasn't been demonstrated. A potential downside of 1Hz heading over UAVCAN is that 87.5~90% of the RTCM traffic on the CAN bus doesn't get used for anything.
@@ -712,12 +712,24 @@ int GPSDriverUBX::configureDevice(const GNSSSystemsMask &gnssSystems) | |||
cfgValset<uint8_t>(UBX_CFG_KEY_CFG_UART2OUTPROT_RTCM3X, 1, cfg_valset_msg_size); | |||
cfgValset<uint32_t>(UBX_CFG_KEY_CFG_UART2_BAUDRATE, uart2_baudrate, cfg_valset_msg_size); | |||
|
|||
// Ublox says to match RTCM TYPES MSM4/MSM7on all ports (issue if combining moving with static base) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue if combining moving with static base
Can you expand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Page 22 of the F9P integration manual has warnings about mixing MSM4 and MSM7 messages on different ports: https://www.u-blox.com/en/docs/UBX-18010802 E.g. The code as is would be using MSM7 from a QGC connected base injected on uart1/I2C and MSM4 over uart2 or uart1 for heading. MSM4 is slightly smaller than MSM7. So I believe it makes sense to use MSM4 for all static and moving baseline corrections. Also, it isn't clear to me that enabling more constellations is helpful for carrier phase based navigation solutions in terms of accuracy or integrity. That was the motivation for wanting to use GNSS mask as a way of enabling GPS only 10Hz rather than the default of 8Hz all constellations enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the doc:
If the receiver is configured to output RTCM messages on several ports, they must all have the
same RTCM configuration otherwise the MSM multiple message bit might not be set properly.
This is true in the case of MSM7 output from a GCS receiver and MSM4 output from a heading base, as they are separate receivers. I don't see any problem here.
Using the config to conditionally enable outputs makes sense otherwise.
Can we get this in? Looks like it should be good to go if the comment questions (eg |
Yes the comments need to be removed, otherwise this is good. |
sync to main before GPS heading v2 merge