Skip to content

Add GPS Status Line to CLI Status Output #12769

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
May 10, 2023
Merged

Add GPS Status Line to CLI Status Output #12769

merged 7 commits into from
May 10, 2023

Conversation

ZzyzxTek
Copy link
Contributor

@ZzyzxTek ZzyzxTek commented May 6, 2023 •
edited
Loading

Note: This is "Part 1" of a series of additions for the GPS Status Line functionality. See [future] comments below for planned additions.

Adds a new GPS: .... status line to the output of the CLI status command.

The GPS: ... status line will appear only when the firmware has been built with the USE_GPS feature define.

If the GPS feature is not enabled, the GPS status line will show GPS: NOT ENABLED.

The GPS status line shows a list of status items following GPS: :

  • Whether Betaflight has established a serial port connection to the GPS module -- either NOT CONNECTED or connected.
  • Serial port information in the format UARTn actualBaudRate (set to configuredBaudRate), for example:
    • UART4 115200 (set to AUTO) when using UART4 connected at 115200 configured with GPS Auto Baud on.
    • UART4 57600 (set to 57600) when using UART4 connected at 57600 configured with GPS Auto Baud off and the serial port configured for 57600.
  • Whether Betaflight has completed its configuration steps -- either auto config OFF, NOT CONFIGURED or configured.
  • [future] Additional actual module configuration items (polled from the module), such as solution rate, platform model, power level, etc. (TBD)

See attached status output screen captures for illustrations of the above output (GPS status is next to last line). (Note the screen shots are a bit out of date vs. the above description.)

Additional Notes:

  • The basic GPS status items implemented for Part 1 of the PR are common for both NMEA and U-BLOX protocols. Testing has shown them to work for both.
  • In testing, it was observed that sometimes when Auto Baud is off the serial port connected at a lower baud rate than configured for the port. Power cycling the GPS module usually fixed this. This not believed to be a bug with this status line code.
  • This PR adds 364 bytes to the firmware size.
  • Betaflight does not currently verify the actual module configuration after completing the configuring steps. The configured status only means the state machine has moved through the configuration steps.
  • Adding additional actual module configuration items requires adding module config polling functionality to the GPS state machine, and will require both NMEA and U_BLOX support.

image
image
image
image

@ZzyzxTek
Add GPS status line to cli status output: connected status, UART + se…
cb84d42 
…rial port baud + configured baud, configured status.
@haslinghuis haslinghuis added this to the 4.5 milestone May 6, 2023
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
haslinghuis
haslinghuis reviewed May 6, 2023
View reviewed changes
haslinghuis
haslinghuis reviewed May 6, 2023
View reviewed changes
haslinghuis
haslinghuis approved these changes May 6, 2023
View reviewed changes
Copy link
Member

@haslinghuis haslinghuis left a comment

Choose a reason for hiding this comment

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

👾

@github-actions

This comment has been minimized.

Sign in to view
@blckmn
Copy link
Member

blckmn commented May 6, 2023 •
edited
Loading

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> PASS
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> FAIL
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> FAIL
  • assigned to an approver -> FAIL
  • approver count at least three -> PASS

@jsk2084
Copy link

jsk2084 commented May 7, 2023

Tested and works OK on a quad using a Beitian BE-220 (M10)...

After turning off auto-baud and setting ports to auto, saw that the gps also connected at 57600 and 115200, for what it's worth.

status

MCU F722 Clock=216MHz, Vref=3.27V, Core temp=48degC
Stack size: 2048, Stack address: 0x20010000
Configuration: CONFIGURED, size: 3690, max available: 16384
Devices detected: SPI:1, I2C:1
Gyros detected: gyro 1 locked dma
GYRO=MPU6000, ACC=MPU6000, BARO=BMP280
OSD: MAX7456 (30 x 16)
BUILD KEY: 66bbb28a739ea41d9cafe4fd941db8ee (4.5.0-zulu)
System Uptime: 30 seconds, Current Time: 2023-05-07T01:30:56.058+00:00
CPU:35%, cycle time: 124, GYRO rate: 8064, RX rate: 15, System rate: 9
Voltage: 1521 * 0.01V (4S battery - OK)
I2C Errors: 0
FLASH: JEDEC ID=0x00ef4018 16M
GPS: connected, UART4 38400(AUTO), configured,
Arming disable flags: RXLOSS CLI MSP

haslinghuis
haslinghuis reviewed May 7, 2023
View reviewed changes
haslinghuis
haslinghuis reviewed May 7, 2023
View reviewed changes
@github-actions

This comment has been minimized.

Sign in to view
ledvinap
ledvinap reviewed May 7, 2023
View reviewed changes
src/main/io/gps.c Outdated
@@ -1985,4 +1985,9 @@ float getGpsDataIntervalSeconds(void)
return gpsDataIntervalSeconds;
}

baudRate_e getGPSPortActualBaudRateIndex(void)
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it enough tu return baudrate, not index?

Copy link
Contributor Author

@ZzyzxTek ZzyzxTek May 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The baud rate index seems to be representation used in a lot of other places. For example, the configured GPS baud rate is represented as an index and a lot of functions in the serial library and elsewhere deal in indexes. I thought it good to maintain consistency with that convention for that getter.

The code in cli.c at lines 4749-4753 prints the two baud rates, so I also thought that code block would look more consistent to deal in indexes and have the two statements perform the similar array lookup, rather than having them use two different approaches.

@ctzsnooze
Copy link
Member

Awesome work!
This will be very helpful for debugging connection errors.
Already just plugging in / unplugging I was able to get an M8 GPS to acquire the wrong baud rate.
Thank you!

@ctzsnooze
Copy link
Member

My only suggestion is perhaps to include the text set to before the value shown for the set speed in brackets, for clarity.

For example,

GPS: connected, UART1 115200 (set to 115200), configured

@sugaarK
Copy link
Member

sugaarK commented May 8, 2023

Good stuff @ZzyzxTek

KarateBrot
KarateBrot requested changes May 8, 2023
View reviewed changes
@ZzyzxTek
Copy link
Contributor Author

ZzyzxTek commented May 8, 2023

My only suggestion is perhaps to include the text set to before the value shown for the set speed in brackets, for clarity.

For example,

GPS: connected, UART1 115200 (set to 115200), configured

Done, thanks!

@ctzsnooze ctzsnooze requested a review from sugaarK May 10, 2023 00:06
@sugaarK
Copy link
Member

sugaarK commented May 10, 2023

I’ve tested this it works good. Very useful

@haslinghuis haslinghuis merged commit 922bc9d into betaflight:master May 10, 2023
haslinghuis pushed a commit to haslinghuis/betaflight that referenced this pull request May 10, 2023
@ZzyzxTek @haslinghuis
Add GPS Status Line to CLI Status Output (betaflight#12769)
b0fd71d 
* Add GPS status line to cli status output: connected status, UART + serial port baud + configured baud, configured status.

* Fixed unit test link fail.

* Really fixed unit test link fail (I hope).

* Really fixed unit test link fail (no really this time).

* Updated to address code reviews.

* Updated to address code reviews.

* Updated to address code reviews.
AkankshaJjw pushed a commit to AkankshaJjw/betaflight that referenced this pull request May 29, 2023
@ZzyzxTek @AkankshaJjw
Add GPS Status Line to CLI Status Output (betaflight#12769)
73b512e 
* Add GPS status line to cli status output: connected status, UART + serial port baud + configured baud, configured status.

* Fixed unit test link fail.

* Really fixed unit test link fail (I hope).

* Really fixed unit test link fail (no really this time).

* Updated to address code reviews.

* Updated to address code reviews.

* Updated to address code reviews.
davidbitton pushed a commit to davidbitton/betaflight that referenced this pull request Feb 5, 2024
@ZzyzxTek @davidbitton
Add GPS Status Line to CLI Status Output (betaflight#12769)
fdc5e67 
* Add GPS status line to cli status output: connected status, UART + serial port baud + configured baud, configured status.

* Fixed unit test link fail.

* Really fixed unit test link fail (I hope).

* Really fixed unit test link fail (no really this time).

* Updated to address code reviews.

* Updated to address code reviews.

* Updated to address code reviews.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ledvinap ledvinap ledvinap left review comments

@haslinghuis haslinghuis haslinghuis approved these changes

@ctzsnooze ctzsnooze ctzsnooze approved these changes

@KarateBrot KarateBrot KarateBrot approved these changes

@sugaarK sugaarK sugaarK approved these changes

Assignees

@ZzyzxTek ZzyzxTek

Labels
RN: IMPROVEMENT
Projects
Finalizing Firmware 4.5 Release
Status: COMPLETED
Milestone
4.5
Development

Successfully merging this pull request may close these issues.
8 participants
@ZzyzxTek @blckmn @jsk2084 @ctzsnooze @sugaarK @ledvinap @haslinghuis @KarateBrot