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

feat: introducing SensorModule to provide access to environmental events #420

Merged
merged 7 commits into from
Nov 6, 2024

Conversation

kschrab
Copy link
Contributor

@kschrab kschrab commented Oct 1, 2024

Description

  • Provides new interfaces to provide vehicle applications with sensor modules.
    • The BasicSensorModule is a single-value based sensor model which can be used to detect icy or rainy areas (e.g., via getOs().getBasicSensorModule().getStrengthOf(SensorType.ICE). This replaces the method getOs().getStateOfEnvironmentSensor(SensorType.ICE) in the general OperatingSystem. The data for this sensor module is produced by the EnvironmentAmbassador.
    • The LidarSensorModule provides access to the previously added PointCloud structure which is produced by vehicle simulators such as PHABMACS or Carla.
    • For now, only VehicleUnits are able to use the new sensor modules.
  • Adjusted all apps (e.g., WeatherWarningApp).

Issue(s) related to this PR

  • Resolves internal issue 889

Affected parts of the online documentation

Changes in the documentation required?

Yes, documentation and Barnim tutorials must be adjusted to use getBasicSensorModule instead now.

Definition of Done

Prerequisites

  • You have read CONTRIBUTING.md carefully.
  • You have signed the Contributor License Agreement.
  • Your GitHub user id is linked with your Eclipse Account.

Required

  • The title of this merge request follows the scheme type(scope): description (in the style of Conventional Commits)
  • You have assigned a suitable label to this pull request (e.g., enhancement, or bugfix)
  • origin/main has been merged into your Fork.
  • Coding guidelines have been followed (see CONTRIBUTING.md).
  • All checks on GitHub pass.
  • All tests on Jenkins pass.

Requested (can be enforced by maintainers)

  • New functionality is covered by unit tests or integration tests. Code coverage must not decrease.
  • If a bug has been fixed, a new unit test has been written (beforehand) to prove misbehavior
  • There are no new SpotBugs warnings.

Special notes to reviewer

@kschrab kschrab self-assigned this Oct 1, 2024
@hoelger
Copy link
Contributor

hoelger commented Oct 1, 2024

had a very brief read and came up with following things:

  • getEnvironmentSensor(): environment here gets a very special naming meaning "coming from environment simulator". also lidar data is from the environment... right? not sure how confusing this is in the future >> getSimpleSensor()
  • getEnvironmentSensor(): singular "Sensor" may be confusing, because environment has multiple sensor types...
  • getOs().getSensorModule().getEnvironmentSensor().enable() : this enable was happening on the module itself (looking at adhoc / cell), so after getModule(), now it's one layer deeper. I find this confusing and for my taste this should be unified.
  • same thing different perspective: with "sensible" you give access to both environment + lidar. so why would you enable() those separately?
  • discussed: have getLidarSensorModule() and getSimpleSensorModule() as two different modules
  • why DefaultSensorModule and not SensorModule (cmp AdhocModule / CellModule) (I don't fully understand why you want to split this very little functionality into the interface SensorModule, this only makes sense if you can reuse it, like for the Sensor<T>). I would merge Default + SensorModule

# Conflicts:
#	app/tutorials/traffic-light-communication/src/main/java/org/eclipse/mosaic/app/tutorial/EventSendingApp.java
#	app/tutorials/weather-warning/src/main/java/org/eclipse/mosaic/app/tutorial/WeatherServerApp.java
#	app/tutorials/weather-warning/src/main/java/org/eclipse/mosaic/app/tutorial/WeatherWarningApp.java
#	fed/mosaic-application/src/main/java/org/eclipse/mosaic/fed/application/ambassador/simulation/AbstractSimulationUnit.java
@kschrab kschrab marked this pull request as ready for review October 9, 2024 12:00
@kschrab kschrab requested a review from rprotzmann October 9, 2024 12:07
@kschrab kschrab added enhancement New feature or request hacktoberfest-accepted labels Oct 9, 2024
@rprotzmann rprotzmann merged commit 55686bb into main Nov 6, 2024
5 checks passed
@kschrab kschrab deleted the 899-sensor-module branch November 12, 2024 08:46
FunKuchen pushed a commit to FunKuchen/mosaic that referenced this pull request Nov 12, 2024
…nts (eclipse#420)

* feat: introducing SensorModule to provide access to environmental events
* feat(application): allow registration of callback for lidar sensor module
* feat(application): re-use Perceptive interface for accessing new sensor modules
FunKuchen pushed a commit to FunKuchen/mosaic that referenced this pull request Nov 12, 2024
…nts (eclipse#420)

* feat: introducing SensorModule to provide access to environmental events
* feat(application): allow registration of callback for lidar sensor module
* feat(application): re-use Perceptive interface for accessing new sensor modules
schwepmo pushed a commit that referenced this pull request Dec 2, 2024
…nts (#420)

* feat: introducing SensorModule to provide access to environmental events
* feat(application): allow registration of callback for lidar sensor module
* feat(application): re-use Perceptive interface for accessing new sensor modules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants