Skip to content
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

State interface input filter #3409

Merged
merged 15 commits into from
Dec 3, 2024
Merged

State interface input filter #3409

merged 15 commits into from
Dec 3, 2024

Conversation

gautierhattenberger
Copy link
Member

@gautierhattenberger gautierhattenberger commented Nov 12, 2024 •
edited
Loading

This PR is a proposal to add a filtering to the state interface setter functions. It allows to have a fine control of the modules allowed to push data to the state. Key elements are:

  • Each module have a unique ID generated at compile time, which can be used to identify the source. In the future, we should consider these values as ID for ABI messages (and remove the hard-coded list).
  • The AHRS primary/secondary selection is not centralised anymore. Each AHRS can be selected by settings or internal function calls. The main benefit is that number of AHRS is not limited. They are either primary (allowed to push data, one at a time) or secondary (not allowed, all the others).
  • The INS don't have settings yet, but only minimal changes are still required to have multiple INS support.
  • The state origin now have getter functions for a cleaner and safer access.

Some remaining work, for this PR or another one:

  • propagate ins_reset with ABI messages (for decoupling purpose, e.g. prepare next point)
  • support multiple INS

@fvantienen @dewagter @Fabien-B What is your opinion on this ?

@gautierhattenberger gautierhattenberger added Airborne New Feature To draft up an idea for a new feature in issue que labels Nov 12, 2024
@fvantienen
Copy link
Member

I think this a very nice improvement especially with multiple INS support! :)
I like the module ID generator, which can be really useful. But I see a problem when replacing the ABI id, since some modules need multiple id's. For example a multi-imu setup like the orange cube has 3 ABI id's for the different IMU's.

@gautierhattenberger
Copy link
Member Author

I'm working on the modifications required for multi-INS, it shouldn't be long.

Your comment on ABI ids is correct. The options I found are:

  • the module requests in the xml file to have more than one ID
  • the ABI id are changed to uint16 (will do that someday anyway) and a part of it is reserved for "components"

@fvantienen do you have a preferred solution ?

@gautierhattenberger
fix wrong ahrs init names
c676dbf
@gautierhattenberger
[ins] decoupled INS implementation
c20f412 
To allow multiple INS at the same time:
- remove weak functions from ins.c
- remove the INS_TYPE_H define
- use ABI message to trigger INS reset
@fvantienen
Copy link
Member

I'm working on the modifications required for multi-INS, it shouldn't be long.

Your comment on ABI ids is correct. The options I found are:

  • the module requests in the xml file to have more than one ID
  • the ABI id are changed to uint16 (will do that someday anyway) and a part of it is reserved for "components"

@fvantienen do you have a preferred solution ?

I think I would prefer the option with the 'components' section of the ID. But both seem fine for me

@gautierhattenberger gautierhattenberger marked this pull request as ready for review November 22, 2024 13:40
@gautierhattenberger
Copy link
Member Author

On a second thought, using senders IDs generated is a bit annoying for some case like IMU calibration where the ABI ID is used. Maybe we can do something with a "component ID" that would be fixed, but then it is not really different from what we have already.

Anyway, the PR should be ready for review.

@fvantienen
Copy link
Member

Will take a look at it later.
The calibration could include the define of the ID and not the number itself maybe? This would solve the problem

@gautierhattenberger
Copy link
Member Author

The calibration could include the define of the ID and not the number itself maybe? This would solve the problem

yes indeed, it is even better to detect some errors at compile time, if the driver is not loaded

fvantienen
fvantienen previously approved these changes Nov 25, 2024
View reviewed changes
Copy link
Member

@fvantienen fvantienen left a comment

Choose a reason for hiding this comment

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

If this is tested I'm ok with merging it :)

@gautierhattenberger
Copy link
Member Author

tested and now working with C++ (EKF)

@gautierhattenberger gautierhattenberger merged commit dfd8e93 into paparazzi:master Dec 3, 2024
1 check passed
@gautierhattenberger gautierhattenberger deleted the state_interface_input_filter branch December 3, 2024 21:34
@gautierhattenberger gautierhattenberger mentioned this pull request Dec 3, 2024
Merged
NoahWe pushed a commit to tudelft/paparazzi that referenced this pull request Dec 11, 2024
@gautierhattenberger @NoahWe
State interface input filter (paparazzi#3409)
0357e1c 
* [generator] generate unique ID and names table for modules
* [state] add accessors for the state origin
* [state] don't access directly to state origin, use getters
* [state] filter inputs with module ID
* [ahrs] convert AHRS modules to state input filter
- selection of ahrs with settings from each module
- init functions for each ahrs modules
- remove old chimu
* update unit test
* [ins] decoupled INS implementation
To allow multiple INS at the same time:
- remove weak functions from ins.c
- remove the INS_TYPE_H define
- use ABI message to trigger INS reset
* [state] protect for c++
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fvantienen fvantienen fvantienen approved these changes

@Fabien-B Fabien-B Awaiting requested review from Fabien-B

Assignees
No one assigned
Labels
Airborne New Feature To draft up an idea for a new feature in issue que
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gautierhattenberger @fvantienen