Skip to content

drivers/gps support ublox basic spectrum analyzer (UBX-MON-SPAN) - #20474

Draft
dagar wants to merge 1 commit into
mainfrom
pr-gps_ubx_mon_span
Draft

drivers/gps support ublox basic spectrum analyzer (UBX-MON-SPAN)#20474
dagar wants to merge 1 commit into
mainfrom
pr-gps_ubx_mon_span

Conversation

@dagar

@dagar dagar commented Oct 25, 2022

Copy link
Copy Markdown
Member

Screenshot from 2022-10-24 21-02-12

@dagar
dagar force-pushed the pr-gps_ubx_mon_span branch from b7b2ce4 to 3772fff Compare October 25, 2022 01:08
@ryanjAA

ryanjAA commented Oct 30, 2022

Copy link
Copy Markdown
Contributor

👍👍

@mrpollo

mrpollo commented Dec 12, 2022

Copy link
Copy Markdown
Contributor

When is this coming in? looks really helpful

@github-actions github-actions Bot added the status:stale Inactive and may be closed. label Apr 10, 2026
@ThomasRigi

Copy link
Copy Markdown
Member

Hi @dagar , while looking into an obscure RTK failure mode that we've encountered, I've found this open PR. We think it would be interesting information (linked to PX4/PX4-GPSDrivers#115 (comment)) . We took your PR as a base, and managed to get some first data quickly.

If there's still interest we can provide support on the PR to get it merged.

One question in particular that we have on this PR is on how GPS_UBX_SPECTRUM is used to set the publication of the data. I would have expected it to also be used in the constructor in the same way as GPS_SAT_INFO, with the rest following analogously:

	/* create satellite info data object if requested */
	if (enable_sat_info) {
		_sat_info = new GPS_Sat_Info();
		_p_report_sat_info = &_sat_info->_data;
		memset(_p_report_sat_info, 0, sizeof(*_p_report_sat_info));
	}

Is there a reason for having two separate ways of activating additional debug topics ?

On a side note (for another PR), we'd also like to add a new message for logging more data from UBX_MSG_MON_RF , in which we'd have the same question on how to init the logging.

# The center frequency at each bin: f(i) = center + span * (i - 127) / 256
uint8[256] spectrum # dB Spectrum data (number of points = span/res)
uint32 spectrum_span_hz # Hz Spectrum span
uint32 resolution_hz # Hz Resolution of the spectrum

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

resolution_hz = spectrum_span_hz / 256, so we can remove this field.

@dakejahl

Copy link
Copy Markdown
Contributor

I think this comes in once we have a way to report it, see mavlink/rfcs#30

@hamishwillee

Copy link
Copy Markdown
Contributor

I think this comes in once we have a way to report it, see mavlink/rfcs#30

@dakejahl Did you confirm that RFC proposes what you want?

@ThomasRigi

Copy link
Copy Markdown
Member

IMO it's already very valuable to have it in the log. For example with Flight Review you can do some fancy stuff:
image

It has already helped us identify a disturbance on a frequency that made the F9P on HPG 1.30 completely lose the fix with RTK activated, and on HPG 1.51 it only saw some minor degradation but kept the RTK fix throughout the disturbance.

We're finalising the code on our branch. I'll be able to provide a PR hopefully in a week. Most of our changes compared to this PR are related to multi-instance logging: we want both L1 and L2 for both our receivers.

Side note: we've also worked on logging all data from the MON-RF topic. It's a very similar change (same for multi-logging). I'll provide a PR too.

@dakejahl

Copy link
Copy Markdown
Contributor

I think this comes in once we have a way to report it, see mavlink/rfcs#30

@dakejahl Did you confirm that RFC proposes what you want?

Mostly yes, still don't quite have time to spend on it.

@ThomasRigi can you re-open this as a fresh PR fully fleshed out? Along with the submodule change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:stale Inactive and may be closed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants