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.

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

add ability to disable board sensors #18550

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PonomarevDA
Copy link
Contributor

Describe problem solved by this pull request
For HITL simulators it is necessary to disable board sensors. At that moment using SYS_HITL=1 parameter you may do it, but it will also set output mode, sensors and commander to the hitl mode that is not what we need for non mavlink based hitl simulators such as uavcan hitl in my case.
Below is how it works now:

if param greater SYS_HITL 0
then
	set OUTPUT_MODE hil
	sensors start -h
	commander start -h
	...

It would be nice to have some way to simply disable all board sensors without additional actions.

Describe your solution
By this PR I would like to start a discussion about extending SYS_HITL params possible states. As an example it might be DISABLE_BOARD_SENSORS, but UAVCAN_HITL is suitable as well.
Here I simply modified the rcS file by adding the new possible state handler.

Describe possible alternatives
I have not discussed with anybody about possible ways to disable board sensors, so it is possible that I missed more easier way to do it. If so, I would appreciate if somebody notice this way. Before adding this state I used to manually comment corresponded board file to disable all sensors, but it was clearly not the way how it may work with clean PX4.

Test data / coverage
Here is an example of flight with uavcan hitl simulator: https://review.px4.io/plot_app?log=baefe706-04b8-4fca-8564-5ed1a981d5e9 with SYS_HITL=3.

@dagar
Copy link
Member

dagar commented Nov 1, 2021

By this PR I would like to start a discussion about extending SYS_HITL params possible states. As an example it might be DISABLE_BOARD_SENSORS, but UAVCAN_HITL is suitable as well.
Here I simply modified the rcS file by adding the new possible state handler.

In general I'd like commander and sensors to be more or less oblivious to HITL (or SITL). Then separately we need to deal with actual physical sensors that should be skipped entirely and output modules. I think capturing the growing number of cases in a single system-wide SYS_HITL isn't going to scale very well. Things like starting sih or another simulator connection should probably already be pushed into appropriate airframes. A lot of these things are just way too specific to be treated generally like this.

This is also related. #18332

@PonomarevDA
Copy link
Contributor Author

I think capturing the growing number of cases in a single system-wide SYS_HITL isn't going to scale very well.

You persuaded me. So, is it suitable to add a new parameter, lets say SYS_BOARD_SENS. If it has non-default value, just ignore running board sensors. Otherwise, I don't see any other way how to disable them even in airframe. If it is ok, I may rename this PR and push a new version.

@dagar
Copy link
Member

dagar commented Nov 2, 2021

SYS_BOARD_SENS

This sounds useful to me for more than just HITL. I nice clean way to opt out of starting all physical sensors without destroying the individual configuration.

We could also try to use it for serial driver startup. #18332

@PonomarevDA
Copy link
Contributor Author

PonomarevDA commented Nov 6, 2021

I've added SYS_BOARD_SENS param and tested it. Here is a flight log in sim.

We could also try to use it for serial driver startup

I didn't get how board sensors (to prevent possible misunderstanding, by board sensors I mean exactly the sensors defined in rc.board_sensors file) are related to the serial driver case. Should this parameter has a bitmask with group of sensors which we should skip: board sensors, everything except mavlink in serial as you suggested in #18332, etc?

@PonomarevDA PonomarevDA changed the title extend SYS_HITL param possible values add ability to disable board sensors Dec 1, 2021
@PonomarevDA
Copy link
Contributor Author

@dagar Hi, could you check this again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement (improvement) 💡 hitl hardware in the loop simulation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants