FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

sbgecom: Implement sbgECom INS driver by tolesam · Pull Request #24137 · PX4/PX4-Autopilot · GitHub

/ PX4-Autopilot Public

sbgecom: Implement sbgECom INS driver - #24137

Merged
dakejahl merged 10 commits into
PX4:mainfrom
SBG-Systems:dev/sbgecom_driver
Sep 11, 2025
ConversationCommits (10)ChecksFiles changed
Merged

sbgecom: Implement sbgECom INS driver#24137
dakejahl merged 10 commits into
PX4:mainfrom
SBG-Systems:dev/sbgecom_driver

Conversation

tolesam commented Dec 19, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

New Feature

Implement sbgECom messages handling to provide IMU sensors, GNSS and EKF data to the autopilot
Be able to parametrize the serial port, baudrate and the communicating mode
Clone sbgECom library only if sbgecom support is enabled and apply a patch
Be able to send SBG Systems INS settings in several ways when starting sbgecom driver

Bug Fixes

Fix sensor airspeed simulator units
Fix HIGHRES_IMU pressure unit

Remarks

Users of external INS (vectornav and sbgecom) need to enable the drivers in the config. As all external INS are currently enabled on some board config, CI is failing.

tolesam changed the title Implement sbgECom INS driver sbgecom: Implement sbgECom INS driver Dec 20, 2024

saengphet commented Dec 21, 2024

Copy link
Copy Markdown

Hello @tolesam I have a question. Does this PR intend to be used with the Ellipse Series, Ellipse 2 Micro Series, or both?

I'm interested in SBG products.

tolesam commented Dec 21, 2024

Copy link
Copy Markdown
Contributor Author

Hello @tolesam I have a question. Does this PR intend to be used with the Ellipse Series, Ellipse 2 Micro Series, or both?

I'm interested in SBG products.

Hello @saengphet,
It's intended to be used with all SBG Systems' INS products.
Feel free to give us any feedback about it, we'll be happy to help you with our products' integration with this driver.
Regards,
Samuel Toledano

tolesam force-pushed the dev/sbgecom_driver branch 2 times, most recently from 72a92c0 to ffe24c4 Compare January 9, 2025 09:16

tolesam commented Jan 10, 2025

Copy link
Copy Markdown
Contributor Author

Hi @dagar ,

Can you take a look at this PR please?
I see you've already integrated another INS driver and we would be happy to have sbgecom driver available on PX4 firmware as well.

Moreover, it seems the job "Build and Push Container" doesn't work on PRs, do you have any idea why?
I tried to run it locally on ubuntu-latest and it worked well.

Thanks in advance.

tolesam force-pushed the dev/sbgecom_driver branch 3 times, most recently from 862fbd7 to 7627a34 Compare January 14, 2025 17:01
bresch previously requested changes Jan 15, 2025
View reviewed changes
Comment thread src/drivers/ins/sbgecom/sbgecom.patch Outdated
tolesam requested a review from bresch January 15, 2025 14:31
tolesam force-pushed the dev/sbgecom_driver branch 5 times, most recently from 5a85ddd to a70bb76 Compare January 27, 2025 16:00
tolesam force-pushed the dev/sbgecom_driver branch from a70bb76 to 7545084 Compare January 31, 2025 16:04
tolesam mentioned this pull request Jan 31, 2025
Closed
tolesam force-pushed the dev/sbgecom_driver branch 4 times, most recently from 667bca1 to ffeede9 Compare February 7, 2025 15:17
tolesam force-pushed the dev/sbgecom_driver branch 4 times, most recently from b227586 to 550b04a Compare February 17, 2025 09:46

tolesam commented Feb 17, 2025

Copy link
Copy Markdown
Contributor Author

Hello @bresch ,
Do you have any feedback about this PR or is there anything I can do to help move it forward?
Thanks in advance for your review.

bresch reviewed Feb 17, 2025
View reviewed changes
Comment thread src/modules/sensors/vehicle_imu/VehicleIMU.cpp Outdated

tolesam commented Aug 8, 2025

Copy link
Copy Markdown
Contributor Author

@mrpollo Great, I've rebased it so board config change is not there anymore.

tolesam force-pushed the dev/sbgecom_driver branch from ec42e10 to 3213f2d Compare August 12, 2025 13:13

mrpollo commented Aug 12, 2025

Copy link
Copy Markdown
Contributor

@tolesam did you mean to keep the *.patch file?

hamishwillee reviewed Aug 13, 2025
View reviewed changes
Comment thread src/drivers/ins/sbgecom/CMakeLists.txt Outdated
hamishwillee previously approved these changes Aug 13, 2025
View reviewed changes

hamishwillee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Approving for the docs part.

tolesam commented Aug 13, 2025

Copy link
Copy Markdown
Contributor Author

@tolesam did you mean to keep the *.patch file?

@mrpollo I've deleted my previous answer which was wrong. I've deleted the patch file.

tolesam force-pushed the dev/sbgecom_driver branch 3 times, most recently from aafc76e to 41a4ffd Compare August 18, 2025 09:37

tolesam commented Aug 18, 2025

Copy link
Copy Markdown
Contributor Author

@mrpollo
It should be good now.
There's still one failing job, I think it's because of missing parameters in the doc but I was told there's a script to update them after merge. Is that correct?

tolesam force-pushed the dev/sbgecom_driver branch 2 times, most recently from c3dee13 to 5cac32c Compare August 19, 2025 12:41

tolesam commented Sep 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Hi @mrpollo , @bresch ,
Is there any update about this PR on your side?
Thanks in advance for the review and your time.

1 similar comment

tolesam commented Sep 8, 2025

Copy link
Copy Markdown
Contributor Author

Hi @mrpollo , @bresch ,
Is there any update about this PR on your side?
Thanks in advance for the review and your time.

dakejahl previously approved these changes Sep 10, 2025
View reviewed changes
Comment thread src/drivers/drv_sensor.h Outdated
Samuel Toledano and others added 10 commits September 10, 2025 21:47
Add new INS driver sbgECom
c2ec6cf 
Implement sbgECom messages handling to provide IMU sensors, GNSS and EKF data to the autopilot
Be able to parametrize the serial port, baudrate and the communicating mode
Clone sbgECom library only if sbgecom support is enabled and apply a patch
Be able to send SBG Systems INS settings in several ways when starting sbgecom driver
Fix sensor airspeed simulator units
028e688
Fix HIGHRES_IMU pressure unit
fecc9f4
Allow HIGHRES_IMU to support 4 IMUs
eedd0fe
sbgECom: Add documentation
8333314
Use submodule instead of fetching sbgECom using CMake
f105e76
Remove patch strategy
723008b
Fix dates
4551532
Remove patch file
6b5aeb0
Update SBG dev type ID
23f1c58 
Co-authored-by: Jacob Dahl <37091262+dakejahl@users.noreply.github.com>

github-actions Bot commented Sep 10, 2025

Copy link
Copy Markdown
Contributor

/en/sensor/sbgecom.md

  • LinkedFileMissingAnchor: #SENS_SBG_CFG not found in ../advanced_config/parameter_reference.md (/home/runner/work/PX4-Autopilot/PX4-Autopilot/docs/en/advanced_config/parameter_reference.md)
  • LinkedFileMissingAnchor: #SBG_BAUDRATE not found in ../advanced_config/parameter_reference.md (/home/runner/work/PX4-Autopilot/PX4-Autopilot/docs/en/advanced_config/parameter_reference.md)
  • LinkedFileMissingAnchor: #SBG_MODE not found in ../advanced_config/parameter_reference.md (/home/runner/work/PX4-Autopilot/PX4-Autopilot/docs/en/advanced_config/parameter_reference.md)
  • LinkedFileMissingAnchor: #SBG_CONFIGURATION_EN not found in ../advanced_config/parameter_reference.md (/home/runner/work/PX4-Autopilot/PX4-Autopilot/docs/en/advanced_config/parameter_reference.md)
  • LinkedFileMissingAnchor: #SBG_BAUDRATE not found in ../advanced_config/parameter_reference.md (/home/runner/work/PX4-Autopilot/PX4-Autopilot/docs/en/advanced_config/parameter_reference.md)

tolesam commented Sep 11, 2025

Copy link
Copy Markdown
Contributor Author

@dakejahl
Thank you for your review and approval yesterday, I've updated this branch with your suggestion and rebased it to fix doc check job which was failing.

hamishwillee commented Sep 11, 2025

Copy link
Copy Markdown
Contributor

The docs flaws can be ignored - they will be fixed in post process. The macOS flaws may be real.

@dakejahl You can merge if you are happy - the docs are OK.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

dakejahl dakejahl approved these changes
bresch Awaiting requested review from bresch
mrpollo Awaiting requested review from mrpollo
hamishwillee Awaiting requested review from hamishwillee

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL