-
Notifications
You must be signed in to change notification settings - Fork 59
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
[tmp] Remove non-API member functions: EntityManager’s TrafficLight related member functions #1329
Closed
TauTheLepton
wants to merge
37
commits into
RJD-1056-remove-npc-logic-started
from
RJD-1057-remove-traffic-lights-from-entity-manager
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Mateusz Palczuk <[email protected]>
…-entity-manager Signed-off-by: Mateusz Palczuk <[email protected]>
…instead of TrafficLightsManager/Supervisor
…dencies on hdmap_utils, improve creation
… simple_sensor_simulator
Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
…move overloads, improve publisher
…ttps://github.com/tier4/scenario_simulator_v2 into RJD-1057-remove-traffic-lights-from-entity-manager
Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
Use pointer for traffic lights Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
… rate updater Signed-off-by: Mateusz Palczuk <[email protected]>
…-entity-manager Signed-off-by: Mateusz Palczuk <[email protected]>
…-entity-manager Signed-off-by: Mateusz Palczuk <[email protected]>
…om-entity-manager' into RJD-1057-remove-traffic-lights-from-entity-manager Signed-off-by: Mateusz Palczuk <[email protected]>
…-entity-manager Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
…ghts**" This reverts commit 87bcb80. Signed-off-by: Mateusz Palczuk <[email protected]>
Signed-off-by: Mateusz Palczuk <[email protected]>
yamacir-kit
deleted the
RJD-1057-remove-traffic-lights-from-entity-manager
branch
September 18, 2024 01:40
yamacir-kit
restored the
RJD-1057-remove-traffic-lights-from-entity-manager
branch
September 27, 2024 02:28
Based on this PR the #1406 has been created |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
DO NOT MERGE | THIS IS A TEMPORARY PR | WHEN THE RIGHT TIME COMES, A CLONE WILL BE CAST TO MASTER
Abstract
This pull request introduces the change of removing any traffic lights functionality from
EntityManager
class and encapsulating it inside the newTrafficLights
class.The responsibility of managing traffic lights have been moved to the API.
The general goal of this PR is to divide scenario_simulator_v2 into smaller modules with some specific functionality and responsibilities. Apart from module dividing, some code has been refactored to make it cleaner, simpler and easier to read.
Background
This pull request is one of many that aim to modularize the scenario_simulator_v2.
Details
The traffic lights functionality has been aggregated in one master
TrafficLights
class. This way, all components that make up traffic lights are grouped together and managed by one resource. This master class - along with the responsibility - has been moved to the API. Although theEntityManager
does hold a reference toTrafficLights
, which is needed by NPCs.Newly developed class
TrafficLights
and its components contain some additional functionality that has been spread throughout the whole scenario_simulator_v2 codebase, like the member functionisRequiredStopTrafficLightState
orgetConventionalTrafficLightsComposedState
.The new traffic lights classes encapsulated by
TrafficLights
are designed in such a way, that they do not share the underlyingTrafficLight
object, but act as a middleman modifying theTrafficLight
object as needed:scenario_simulator_v2/simulation/traffic_simulator/include/traffic_simulator/traffic_lights/traffic_lights_base.hpp
Lines 60 to 73 in a06f4b4
Some additional traffic light message type conversions have been developed in order to get rid of the
simulation_api_schema
dependency intraffic_simulator
and make message managing easier. Also, the traffic lights are now responsible for generating the messages:scenario_simulator_v2/simulation/traffic_simulator/include/traffic_simulator/traffic_lights/traffic_lights_base.hpp
Lines 75 to 80 in a06f4b4
Additionally, a dedicated message publisher has been developed for
simple_sensor_simulator
:scenario_simulator_v2/simulation/simple_sensor_simulator/include/simple_sensor_simulator/sensor_simulation/traffic_lights/traffic_lights_publisher.hpp
Lines 27 to 53 in a06f4b4
Please note that this PR removes all tests for
TrafficLightManager
as these tests have no purpose with the newTrafficLights
implementation.New tests have been developed on this branch (here is a diff), but they have not been added to this PR, as this is a large PR by itself as is now.
We are planning to cast a new PR with just the tests. However, if the reviewer would like to have the tests included in this PR as well, we can do that, just please let us know.
Please just note, that then (with tests included) this PR will become very large with over 2000 additions (tests alone have 1000 additions).
Additional note: many of the changes are restructuring, so the code is changed only a little, if at all, but it has been moved to some other places like for example this has been moved here.
Likewise, these type conversions have been moved from
publish
member functions to appropriate traffic lights functions, which use cast conversions defined here.References
INTERNAL LINK
Destructive Changes
None
Known Limitations
None