add an option to enable Galileo for UBLOX #2583

stronnag · 2017-12-13T21:04:57Z

As yet untested (other than seeing that the Cli option is present)

stronnag · 2017-12-13T21:45:14Z

Currently, this allocates GNSS channels as:

Id	GNSS	Min channels	Max channels	Enabled
0	GPS	8	16	Y
1	SBAS	1	3	Y
2	Galileo	4	8	Y
3	BeiDou	8	16
4	IMES	0	8
5	QZSS	0	3
6	GLONASS	8	14	Y

Usually gives me between 24 and 27 satellites.

giacomo892 · 2017-12-14T09:27:07Z

Why do you need to use UBLOX7? I'm using GALILEO as configured via u-center and it is working perfectly at 5Hz.

stronnag · 2017-12-14T09:34:35Z

Expedience mainly. No reason other than I've not yet got further in defining the exact criterion (e.g. GPS firmware version). One of the several reasons why it is currently marked "Do not merge".

giacomo892 · 2017-12-14T10:01:08Z

I've a BN880, among the others, that like 50% of the times fails to be auto-configured with UBLOX7 at boot time and it falls back to standard UBLOX at 5Hz. Do you ever experienced this? So that module is likely to miss GALILEO most of the times. I think I should configure it via u-center and don't use auto-configure on it.. but :P

stronnag · 2017-12-14T10:18:20Z

Ucenter doesn't help users with units without non-volatile storage. It is not auto-configuration; it is user preference via the CLI, with a sanity check. If your unit is broken, don't use it. It uses exactly the same setup stanza as ucenter, so is as likely to be successful.

giacomo892 · 2017-12-14T10:58:36Z

Maybe I don't have explained the issue clearly. Configuration changes that I do via u-center are sticky. My sole issue with that module if that if I choose UBLOX7 protocol in the configuration tab, I'm getting 5Hz updates via POSLLH most of the times instead of getting NAV-PVT at 10Hz. It like an auto-configuration issue. If I enable NAV-PVT from u-center and disable auto-configuration in INAV of all works just fine at 10Hz all the times.

Changed the CLI variable to `gps_ublox_use galileo` for consistency with other GPS variables Changed the eligibility test to M8N hardware and v3 firmware (or later)

stronnag · 2017-12-14T18:30:10Z

Tested with M8N, Galileo capable but not enabled in EEPROM.

With gps_ublox_use_galileo = OFF, Galileo not enabled
With gps_ublox_use_galileo = ON, Galileo is enabled

stronnag · 2017-12-14T21:32:30Z

Fortuitously, the hacky version selection will work on other user's devices ....

fiam · 2017-12-15T16:21:30Z

src/main/fc/settings.yaml

    enum: gpsDynModel_e
+  - name: gps_ublox_use_galileo
+    values: ["OFF", "ON"]
+    enum: gpsUbloxUseGalileo_e


Can you remove this table? It seems it does nothing now.

I think the error is that gps_ublox.c no longer references the related configure variable, which was not intended when I changed the criteria.

If I'm understanding it correctly, a boolean without any table seems like the best option. Do you plan to add more states to the variable in the future?

stronnag · 2017-12-15T18:48:32Z

No just following similar examples. Please educate me / update the PR if there is a better way

fiam · 2017-12-16T11:28:09Z

@stronnag

TLDR: Your setting definition is correct. There's just a small issue with the field definition in the .h file.

I really need to document the settings file format better, sorry about that. For now, let me briefly explain the concepts relevant to this PR:

Each setting must have a type. It can be explicit (defined in settings.yaml via type: field) or implicit (determined by parsing the .h files by the generator).
Settings with numeric values define a minimum and maximum values. The settings machinery uses these values internally to validate values set by the user (e.g. CLI command set foo = 7). If any of those values is 0, it can be omitted (that's why you see some settings with just max and no min).
Settings based on enums require a table which indicates the name of each value. Its minimum and maximum values are defined as 0 and number_of_values_in_enum - 1 respectively for these ones. Tables are defined at the top of the settings file, in the tables: section and then referenced by name via the table: field in the setting definition. This means several settings can share the same table.
In INAV, boolean settings are represented as enum settings with a table mapping 0 to OFF and 1 to ON.
Which brings us to the last point for today. There are so many boolean settings that manually referencing the ON/OFF table became tedious, so I introduced a shorthand for them by setting the type to bool (i.e. type: bool in the setting definition). This sets the type as an uint8_t and the table to ON/OFF.

fiam · 2017-12-16T11:31:42Z

src/main/io/gps.h

    gpsAutoConfig_e autoConfig;
    gpsAutoBaud_e autoBaud;
    gpsDynModel_e dynModel;
+    gpsUbloxUseGalileo_e ubloxUseGalileo;


I'd just use a boolean here. Having an enum for just 0 and 1 complicates the code and it seems unlikely that we'll want to add more states to this field in the future.

fiam · 2017-12-16T11:37:11Z

@stronnag It seems you forgot to initialise the field when resetting the PG. You need to set its default value around line 134 in gps.c:

inav/src/main/io/gps.c

Line 134 in 652a309

PG_RESET_TEMPLATE(gpsConfig_t, gpsConfig,

stronnag · 2017-12-16T11:41:33Z

I think I will write an 'adding a new CLI option' wiki page after this.

…t, improve in code documentation

stronnag · 2018-01-19T15:19:16Z

As this has been independently tested on a new 'no eeprom' GPS, I'm merging.

add an option to enable Galileo for UBLOX

65c6887

stronnag added Don't merge Enhancement labels Dec 13, 2017

fix max config size of ublox galileo setting

450860e

UBLOX Galileo enablement:

25be37e

Changed the CLI variable to `gps_ublox_use galileo` for consistency with other GPS variables Changed the eligibility test to M8N hardware and v3 firmware (or later)

stronnag removed the Don't merge label Dec 14, 2017

stronnag added 2 commits December 14, 2017 21:40

Merge branch 'development' into ublox-enable-galileo

bcda921

Merge branch 'development' into ublox-enable-galileo

0ee56e9

fiam reviewed Dec 15, 2017

View reviewed changes

restore missing check on CLI setting (agh review comment 1)

60f6a9d

second attempt at understanding how settings work

652a309

fiam reviewed Dec 16, 2017

View reviewed changes

ubloxGalileo : clean up of data types, initialisation, message conten…

26c340b

…t, improve in code documentation

stronnag merged commit 76a09c9 into development Jan 19, 2018

stronnag deleted the ublox-enable-galileo branch January 19, 2018 15:19

digitalentity added this to the 1.9 milestone Jan 21, 2018

DzikuVx mentioned this pull request Feb 5, 2018

Galileo GNSS configuration missing #1345

Closed

add an option to enable Galileo for UBLOX #2583

add an option to enable Galileo for UBLOX #2583

Uh oh!

Conversation

stronnag commented Dec 13, 2017

Uh oh!

stronnag commented Dec 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

giacomo892 commented Dec 14, 2017

Uh oh!

stronnag commented Dec 14, 2017

Uh oh!

giacomo892 commented Dec 14, 2017

Uh oh!

stronnag commented Dec 14, 2017

Uh oh!

giacomo892 commented Dec 14, 2017

Uh oh!

stronnag commented Dec 14, 2017

Uh oh!

stronnag commented Dec 14, 2017

Uh oh!

fiam Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

stronnag Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

fiam Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

stronnag commented Dec 15, 2017

Uh oh!

fiam commented Dec 16, 2017

Uh oh!

fiam Dec 16, 2017

Choose a reason for hiding this comment

Uh oh!

fiam commented Dec 16, 2017

Uh oh!

stronnag commented Dec 16, 2017

Uh oh!

stronnag commented Jan 19, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

stronnag commented Dec 13, 2017 •

edited

Loading