feat(charging): provide the charging stations within a certain area #446

kschrab · 2024-12-12T10:42:22Z

Description

New method to provide a list of available charging stations within the vicinity of the vehicle (only for Electric Vehicles).

Issue(s) related to this PR

Resolves internal issue 950

Affected parts of the online documentation

Changes in the documentation required?

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

...n/java/org/eclipse/mosaic/fed/application/ambassador/util/EventNicenessPriorityRegister.java

.../main/java/org/eclipse/mosaic/interactions/electricity/ChargingStationDiscoveryResponse.java

hoelger · 2024-12-12T16:22:22Z

...java/org/eclipse/mosaic/interactions/electricity/VehicleChargingStationDiscoveryRequest.java

+    }
+
+    @Serial
+    private static final long serialVersionUID = 1L;


Why do we have the serial number here? If it's an interaction-specific thing, why don't we use it in the response as well?

Interaction implements Serializable, if we would do distributed simulation in some way or another and serialize/deserialize interactions between remote hosts, this UID would be used to assure that all sides have the same class version loaded. When adding new Interactions giving a serialVersionUID of 1 is good, indicating version 1.

Now what we should consider though is that we update this ID whenever we change Interactions in a meaningful manner.

(see https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/Serializable.html)

I hope this suffices as a explanation 😅

A specific version is needed especially when storing the serialized objects in files or databases and reading them later. Then the version plays a crucial role to determine if a previously stored object is compatible for deserialization or not. Since we don't do such things, we decided long ago that we use 1 always and everywhere where we implement Serializable.

We transfer interactions between PHABMACS and MOSAIC by using this object-serialization, thus we need to implement the Serializable interface for all interactions, and for all objects which are used by interactions. Nevertheless, since we serialize/deserialize the objects just for the transport from/to PHABMACS, we don't need to care about versioning, thus using version 1 again everywhere.

...ation/src/main/java/org/eclipse/mosaic/fed/application/ambassador/ApplicationAmbassador.java

schwepmo · 2024-12-17T10:28:10Z

.gitignore

+.tiles
+/logs/


This should not be necessary, please remove

@iwillitried yes, there is another .gitignore in mosaic-starter which ignores the logs directory. That should suffice and you can remove the /logs/ entry here.

.../main/java/org/eclipse/mosaic/fed/application/ambassador/simulation/ElectricVehicleUnit.java

schwepmo · 2024-12-17T10:39:59Z

...java/org/eclipse/mosaic/interactions/electricity/VehicleChargingStationDiscoveryRequest.java

+    }
+
+    @Serial
+    private static final long serialVersionUID = 1L;


Interaction implements Serializable, if we would do distributed simulation in some way or another and serialize/deserialize interactions between remote hosts, this UID would be used to assure that all sides have the same class version loaded. When adding new Interactions giving a serialVersionUID of 1 is good, indicating version 1.

Now what we should consider though is that we update this ID whenever we change Interactions in a meaningful manner.

(see https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/Serializable.html)

I hope this suffices as a explanation 😅

.../main/java/org/eclipse/mosaic/interactions/electricity/ChargingStationDiscoveryResponse.java

.../main/java/org/eclipse/mosaic/fed/application/app/api/os/ElectricVehicleOperatingSystem.java

…#965)

...n/java/org/eclipse/mosaic/fed/application/ambassador/util/EventNicenessPriorityRegister.java

...ic-objects/src/main/java/org/eclipse/mosaic/lib/objects/electricity/ChargingStationData.java

.../main/java/org/eclipse/mosaic/fed/application/app/api/os/ElectricVehicleOperatingSystem.java

...pse/mosaic/fed/application/ambassador/simulation/electric/objects/ChargingStationObject.java

...pse/mosaic/fed/application/ambassador/simulation/electric/providers/ChargingStationTree.java

hoelger · 2025-01-23T13:59:39Z

...pse/mosaic/fed/application/ambassador/simulation/electric/providers/ChargingStationTree.java

+public class ChargingStationTree extends ChargingStationIndex {
+    private final int bucketSize;
+
+    private KdTree<ChargingStationObject> chargingStationTree;


I don't find the code where this Tree is updated... can you help me? The tree should be updated (= take all data from indexedChargingStations and put it into the tree) latest every time before a search is issued?

...n/java/org/eclipse/mosaic/fed/application/ambassador/util/EventNicenessPriorityRegister.java

hoelger · 2025-01-23T15:20:54Z

...pse/mosaic/fed/application/ambassador/simulation/electric/providers/ChargingStationTree.java

+    }
+
+    @Override
+    public void onChargingStationUpdate() {


I know that for e.g. TrafficLightTree this does not exist, but please add tests for the functions in ChargingStationIndex and ChargingStationTree

- Merged contents of ChargingStationTree into ChargingStationIndex - ChargingStationIndex is not abstract anymore - small changes regarding function naming and removal of empty lines

kschrab

Minor changes required. Also, please check if all unit tests are green, currently, ApplicationAmbassadorTest.processInteraction_ChargingStationRegistration fails

kschrab · 2025-02-03T11:37:24Z

...se/mosaic/fed/application/ambassador/simulation/electric/providers/ChargingStationIndex.java

+                .setChargingStationData(chargingStationData);
+    }
+
+    public List<ChargingStationObject>getChargingStationsInCircle(GeoCircle circle) {


Code style: missing space before method name

kschrab · 2025-02-03T11:40:18Z

...se/mosaic/fed/application/ambassador/simulation/electric/providers/ChargingStationIndex.java

+    public void onChargingStationUpdate() {
+        if (chargingStationTree == null) { // initialize before first update is called
+            List<ChargingStationObject> allChargingStations = new ArrayList<>(indexedChargingStations.values());
+            chargingStationTree = new KdTree<>(new SpatialObjectAdapter<>(), allChargingStations, bucketSize);
+            treeTraverser = new SpatialTreeTraverser.InRadius<>();
+        }
+    }


Does this need to be public? I would make that private, rename it to initSearchTree() and move it right below updateChargingStation method where it is needed

kschrab · 2025-02-03T11:40:47Z

...se/mosaic/fed/application/ambassador/simulation/electric/providers/ChargingStationIndex.java

+    public void initialize() {
+        // initialization at first update
+    }


Why do we have this method if we don't do anything here?

kschrab · 2025-02-03T11:41:25Z

.../main/java/org/eclipse/mosaic/fed/application/ambassador/simulation/ElectricVehicleUnit.java

+    public List<ChargingStationData> getChargingStationsInArea(GeoCircle searchArea) {
+        return SimulationKernel.SimulationKernel.getChargingStationIndex().getChargingStationsInCircle(searchArea)
+                .stream().map(ChargingStationObject::getChargingStationData)
+                .collect(Collectors.toList());


use toList() instead of .collect(Collectors.toList());

kschrab · 2025-02-03T11:42:00Z

...pplication/src/main/java/org/eclipse/mosaic/fed/application/ambassador/SimulationKernel.java

+        }
+
+        return this.chargingStationIndex;
+    }


code style: missing blank line after this method.

…ingStationRegistration(...)

provide the charging stations within a certain area

e934a58

hoelger suggested changes Dec 12, 2024

View reviewed changes

schwepmo reviewed Dec 16, 2024

View reviewed changes

...ation/src/main/java/org/eclipse/mosaic/fed/application/ambassador/ApplicationAmbassador.java Outdated Show resolved Hide resolved

schwepmo requested changes Dec 17, 2024

View reviewed changes

ChargingStations discovery is synchronous (related to mosaic-extended…

434bb9a

…#965)

kschrab commented Jan 21, 2025

View reviewed changes

kschrab added the enhancement New feature or request label Jan 21, 2025

kschrab marked this pull request as ready for review January 21, 2025 12:58

resolving pull request issues

b7ad846

hoelger reviewed Jan 23, 2025

View reviewed changes

hoelger suggested changes Jan 23, 2025

View reviewed changes

iwillitried added 2 commits January 29, 2025 11:12

Merge branch 'main' into 950-Provide-Charging-Stations-around-a-GeoPoint

83137fa

- Removed the file: ChargingStationTree.java

a0cf89b

- Merged contents of ChargingStationTree into ChargingStationIndex - ChargingStationIndex is not abstract anymore - small changes regarding function naming and removal of empty lines

kschrab commented Feb 3, 2025

View reviewed changes

iwillitried added 2 commits February 4, 2025 12:08

resolving PR issues

0d05e63

Fixing Unit tests: ApplicationAmbassadorTest.processInteraction_Charg…

fb7ce5a

…ingStationRegistration(...)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(charging): provide the charging stations within a certain area #446

feat(charging): provide the charging stations within a certain area #446

kschrab commented Dec 12, 2024 •

edited

Loading

hoelger Dec 12, 2024

schwepmo Dec 17, 2024

kschrab Feb 3, 2025

schwepmo Dec 17, 2024

kschrab Feb 3, 2025

schwepmo Dec 17, 2024

hoelger Jan 23, 2025

hoelger Jan 23, 2025

kschrab left a comment

kschrab Feb 3, 2025

kschrab Feb 3, 2025

kschrab Feb 3, 2025

iwillitried Feb 4, 2025

kschrab Feb 3, 2025

iwillitried Feb 4, 2025

kschrab Feb 3, 2025

iwillitried Feb 4, 2025

		.tiles
		/logs/

feat(charging): provide the charging stations within a certain area #446

Are you sure you want to change the base?

feat(charging): provide the charging stations within a certain area #446

Conversation

kschrab commented Dec 12, 2024 • edited Loading

Description

Issue(s) related to this PR

Affected parts of the online documentation

Definition of Done

Special notes to reviewer

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kschrab left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kschrab commented Dec 12, 2024 •

edited

Loading