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

Feature: failure detector hitl #666

Merged
merged 7 commits into from
May 8, 2024
Merged

Conversation

Ilia-Loginov
Copy link

@Ilia-Loginov Ilia-Loginov commented Apr 5, 2024

Component: Introduced failure_detector_hitl module, integrating three classes for handling failure injection messages(FailureDetectorHITL),

simulating sensors that have ceased to update (FakeStuckSensor), and a component managing a collection of these simulated sensors (FakeSensors).

This module is a combination of three classes: FailureDetectorHITL for detecting failure injection messages, FakeStuckSensor for simulating sensors whose readings have become stuck, and FakeSensors, a component that aggregates a collection of such simulated sensors.

Additionally, the handling of the sensors module has been extended to support disabling sensors during flight.

Designed exclusively for operation in HITL (Hardware-In-The-Loop) mode, this module does not interfere with normal operational mode.

Reported-by: Ilia Loginov [email protected]

On branch FEATURE_FAILURE_INJECTION_HITL
Changes to be committed:
modified: src/modules/sensors/CMakeLists.txt
new file: src/modules/sensors/failure_detector_HITL/CMakeLists.txt
new file: src/modules/sensors/failure_detector_HITL/failure_detector_HITL.cpp
new file: src/modules/sensors/failure_detector_HITL/failure_detector_HITL.hpp
modified: src/modules/sensors/sensors.cpp
modified: src/modules/sensors/sensors.hpp

Tested on Pixhawk and Saluki v3 HITL mode

… classes for handling failure injection messages(FailureDetectorHITL),

simulating sensors that have ceased to update (FakeStuckSensor), and a component managing a collection of these simulated sensors (FakeSensors).

This module is a combination of three classes: FailureDetectorHITL for detecting failure injection messages,
FakeStuckSensor for simulating sensors whose readings have become stuck, and FakeSensors, a component that aggregates a collection of such simulated sensors.

Additionally, the handling of the sensors module has been extended to support disabling sensors during flight.

Designed exclusively for operation in HITL (Hardware-In-The-Loop) mode, this module does not interfere with normal operational mode.

Reported-by: Ilia Loginov [email protected]

On branch FEATURE_FAILURE_INJECTION_HITL
Changes to be committed:
	modified:   src/modules/sensors/CMakeLists.txt
	new file:   src/modules/sensors/failure_detector_HITL/CMakeLists.txt
	new file:   src/modules/sensors/failure_detector_HITL/failure_detector_HITL.cpp
	new file:   src/modules/sensors/failure_detector_HITL/failure_detector_HITL.hpp
	modified:   src/modules/sensors/sensors.cpp
	modified:   src/modules/sensors/sensors.hpp
@Ilia-Loginov Ilia-Loginov changed the title Component: Introduced failure_detector_hitl module, integrating three… Feature: failure detector hitl Apr 5, 2024
Comment on lines 164 to 179
if (_hil_enabled) {
#if defined(CONFIG_SENSORS_VEHICLE_GPS_POSITION)
InitializeVehicleGPSPosition();
#endif // CONFIG_SENSORS_VEHICLE_GPS_POSITION

#if defined(CONFIG_SENSORS_VEHICLE_AIR_DATA)
InitializeVehicleAirData();
#endif // CONFIG_SENSORS_VEHICLE_AIR_DATA

#if defined(CONFIG_SENSORS_VEHICLE_MAGNETOMETER)
InitializeVehicleMagnetometer();
#endif // CONFIG_SENSORS_VEHICLE_MAGNETOMETER

_fakeSensors.update(_failureDetector);
}

Copy link

@haitomatic haitomatic Apr 8, 2024

Choose a reason for hiding this comment

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

shall we move this block to be after this block at L256 ? and remove all the InitalizeVehicle* since there is now double Initialization in hitl case ?

#if defined(CONFIG_SENSORS_VEHICLE_AIR_DATA)
	InitializeVehicleAirData();
#endif // CONFIG_SENSORS_VEHICLE_AIR_DATA
#if defined(CONFIG_SENSORS_VEHICLE_GPS_POSITION)
	InitializeVehicleGPSPosition();
#endif // CONFIG_SENSORS_VEHICLE_GPS_POSITION
#if defined(CONFIG_SENSORS_VEHICLE_MAGNETOMETER)
	InitializeVehicleMagnetometer();
#endif // CONFIG_SENSORS_VEHICLE_MAGNETOMETER
#if defined(CONFIG_SENSORS_VEHICLE_OPTICAL_FLOW)
	InitializeVehicleOpticalFlow();
#endif // CONFIG_SENSORS_VEHICLE_OPTICAL_FLOW

Copy link
Author

@Ilia-Loginov Ilia-Loginov Apr 15, 2024

Choose a reason for hiding this comment

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

Unfortunately no, because this block should work in amed_ case and be able to change status of sensors during a fly.

	if (_armed) {
		return 0;
	}

However, I can refactor next logic.

	if (!_armed) {
		/* All next code */
	}
	
	if (_hil_enabled || !_armed) {
#if defined(CONFIG_SENSORS_VEHICLE_GPS_POSITION)
		InitializeVehicleGPSPosition();
#endif // CONFIG_SENSORS_VEHICLE_GPS_POSITION

#if defined(CONFIG_SENSORS_VEHICLE_AIR_DATA)
		InitializeVehicleAirData();
#endif // CONFIG_SENSORS_VEHICLE_AIR_DATA

#if defined(CONFIG_SENSORS_VEHICLE_MAGNETOMETER)
		InitializeVehicleMagnetometer();
#endif // CONFIG_SENSORS_VEHICLE_MAGNETOMETER

		if (!_armed) {
#if defined(CONFIG_SENSORS_VEHICLE_OPTICAL_FLOW)
		    InitializeVehicleOpticalFlow();
#endif // CONFIG_SENSORS_VEHICLE_OPTICAL_FLOW
              }
              
		_fakeSensors.update(_failureDetector);
	}

@Ilia-Loginov Ilia-Loginov self-assigned this Apr 17, 2024
@haitomatic haitomatic force-pushed the FEATURE_FAILURE_INJECTION_HITL branch from 3e2eb78 to 1f2a849 Compare April 22, 2024 13:21
@haitomatic
Copy link

some refactoring:

  • make it clear for ok/off/stuck with isSensorOk()/isSensorOff()/IsSensorStuck()
  • Simplify the update of _fakeSensors in Sensors::parameters_update() . _fakeSensors,Update() can be use also for turnOffAll()
  • Update stuck msg timestamp with current time (timestamp_sample as part of the sample remains static (stuck))

@haitomatic haitomatic requested a review from jlaitine April 23, 2024 06:07
@haitomatic haitomatic force-pushed the FEATURE_FAILURE_INJECTION_HITL branch 2 times, most recently from 2da5db9 to 5fd2c8c Compare April 23, 2024 14:47
…uck) sensor publishers instead of stop / restart
@haitomatic haitomatic force-pushed the FEATURE_FAILURE_INJECTION_HITL branch from 5fd2c8c to daa03e3 Compare April 23, 2024 14:52
@jlaitine
Copy link

jlaitine commented May 3, 2024

I added a few comments, maybe I need to understand the thing better still. Code looks nice and clean, no objections to that.

However, I would like to see in the code

  1. whether a failure detector (this HITL one or some other) is supposed to run only in HITL, or always
  2. if it is only in HITL, it should be better separated from the flying configuration, i.e. not initialize / call failure detector or fake sensors functions when flying.
  3. if it is for SITL, HITL and flying cases, then the framework should support adding different failure detectors, initializing the proper one based on the build mode / HITL mode at boot and running the functions always. Initial failure detector for non-HITL case could be just a stub that always returns OK for the sensors.

@haitomatic
Copy link

haitomatic commented May 7, 2024

@Ilia-Loginov , I just add all the hitl mode checks . As we agree now that this failure injection should only run in HITL mode

@Ilia-Loginov
Copy link
Author

@Ilia-Loginov , I just add all the hitl mode checks . As we agree now that this failure injection should only run in HITL mode

Thank you!

@Ilia-Loginov Ilia-Loginov reopened this May 8, 2024
@haitomatic haitomatic requested a review from jlaitine May 8, 2024 07:31
Copy link

@jlaitine jlaitine left a comment

Choose a reason for hiding this comment

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

LGTM

@Ilia-Loginov Ilia-Loginov marked this pull request as ready for review May 8, 2024 09:55
@Ilia-Loginov Ilia-Loginov merged commit cb9433a into main May 8, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants