feat(interaction, module): separated lidar from veh updates, new Lidar module #440
Conversation
|
|
||
| @Override | ||
| public void reactOnSensorUpdate(Consumer<PointCloud> callback) { | ||
| this.callback = callback; |
There was a problem hiding this comment.
Maybe allow more than one callback here (make this.callback a list)
|
|
||
| @Override | ||
| public void disable() { | ||
| this.enabled = false; |
There was a problem hiding this comment.
Should we send a VehcielSensorActivation here to deactivate Lidar scanning in the simulators?
| .isEquals(); | ||
| } | ||
|
|
||
| //TODO what does this do? |
There was a problem hiding this comment.
This is for logging/printing the interaction.
| /** | ||
| * List of {@link PointCloud} containing LiDAR data from the simulator. | ||
| */ | ||
| private Map<String, PointCloud> updated = new HashMap<>(); |
There was a problem hiding this comment.
Why do we need a map here? Could be a list instead.
List<Pair<String, PointCloud>>
or
public record LidarUpdate(String vehicleOd, PointCloud pointCloud) {}
List<LidarUpdates.LidarUpdate> updates;
| public String toString() { | ||
| return new ToStringBuilder(this, SHORT_PREFIX_STYLE) | ||
| .appendSuper(super.toString()) | ||
| .append("updated", this.updated) |
There was a problem hiding this comment.
Don't just append the map here. I'm not even sure what happens if you just put the map here, I guess it would just print HashMap@a78f8e9b or something like that. Better append something meaningful, such as
return new ToStringBuilder(this, SHORT_PREFIX_STYLE)
.appendSuper(super.toString())
.append("updated", "Updated Lidar data for vehicles [" + /*TODO collect vehicle ids*/ + "]);
| return this.currentPointcloud; | ||
| } | ||
|
|
||
| public void addLidarUpdate(PointCloud pointCloud) { |
There was a problem hiding this comment.
Maybe rename to updatePointCloud. add.. sounds like that this module stores more than one pointcloud object.
| @@ -62,7 +62,7 @@ buildNumber.properties | |||
|
|
|||
| # MOSAIC | |||
|
|
|||
| tmp/ | |||
| @@ -693,6 +697,24 @@ private void process(final VehicleUpdates vehicleUpdates) { | |||
| addEvent(triggerGarbageCollection); | |||
| } | |||
|
|
|||
| private void process(final LidarUpdates lidarUpdates) { | |||
| for (LidarUpdates.LidarUpdate lidarUpdate :lidarUpdates.getUpdated()) { | |||
There was a problem hiding this comment.
there is a space missing after the :
| private long nextUpdate; | ||
|
|
||
|
|
||
| public record LidarUpdate(String unitId, PointCloud pointCloud) {} |
There was a problem hiding this comment.
What do we think about this style-wise @kschrab? Typically, we placed in-line classes at the bottom of a class what should we do with records?
| @@ -99,6 +103,7 @@ public VehicleUnit(String vehicleName, VehicleType vehicleType, final GeoPoint i | |||
| } | |||
|
|
|||
| basicSensorModule = new EnvironmentBasicSensorModule(); | |||
| defaultLidarSensorModule = new DefaultLidarSensorModule(this.getId()); | |||
There was a problem hiding this comment.
How would we switch to a non-default LidarSensorModule?
| VehicleSensorActivation interaction = new VehicleSensorActivation( | ||
| SimulationKernel.SimulationKernel.getCurrentSimulationTime(), | ||
| unitId, | ||
| 0, |
There was a problem hiding this comment.
So deactivation just happens by setting the range to 0? Would a VehicleSensorDeactivation not cleaner?
Description
The following changes were made on this branch
A new
LidarUpdatesinteraction was addedThe
ApplicationAmbassadorwas refactored to use this new interactionA new implementation of
LidarSensorModulewas created namedDefaultLidarSensorModuleThe new
LidarSensorModuleis being used where LiDAR data was previously received through vehicle updatesThe
VehicleUnitnow has the possibility to return the new LidarSensorModule, making LiDAR data accessible to unitsNot related to this, the gitignore now includes the ./tiles folder that saves downloaded osm sattelite/map data
Issue(s) related to this PR
Affected parts of the online documentation
Definition of Done
Prerequisites
Required
type(scope): description(in the style of Conventional Commits)enhancement, orbugfix)origin/mainhas been merged into your Fork.Requested (can be enforced by maintainers)
Special notes to reviewer