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

RJD-1057 (2/5): Remove non-API member functions: EntityManager’s TrafficLight related member functions (tests) #1444

Draft
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

dmoszynski
Copy link
Contributor

@dmoszynski dmoszynski commented Nov 8, 2024

Description

Abstract

This pull request is a continuation of #1406.
It adds tests for the new implementation of the traffic lights.

After changes requested review in #1406 and other:
In order to make it possible to test the correctness of the generation the messages:

  • autoware_auto_perception_msgs::msg::TrafficSignalArray
  • autoware_perception_msgs::msg::TrafficSignalArray
  • autoware_perception_msgs::msg::TrafficLightGroupArray
  • traffic_simulator_msgs::msg::TrafficLightArrayV1

from simulation_api_schema::UpdateTrafficLightsRequest, new static method has been extracted:

static auto generateMessage(
const rclcpp::Time &, const simulation_api_schema::UpdateTrafficLightsRequest & request,
const std::string & frame = "") -> std::unique_ptr<MessageType>;

In traffic_light_publisher.cpp can be found definitions of methods used in tests.

Background

This pull request is one of many that aim to modularize the scenario_simulator_v2.

Details

In the previous pull request, a new implementation of traffic lights has been added and tests developed for the previous implementation have been deleted.
This pull request introduces tests for the new implementation of traffic lights. These changes have been split into two PRs, because of the amount of changes. If these changes (this PR and the previous one) were introduced in one PR, it would have been very large (>2000 additions).

References

INTERNAL LINK

Destructive Changes

Known Limitations

TauTheLepton and others added 8 commits July 29, 2024 12:41
This reverts commit 433019c.

Signed-off-by: Mateusz Palczuk <[email protected]>
…o RJD-1057-traffic-lights-tests

Signed-off-by: Mateusz Palczuk <[email protected]>
…om-entity-manager' into RJD-1057-traffic-lights-tests

Signed-off-by: Mateusz Palczuk <[email protected]>
…rom-entity-manager' into RJD-1057-traffic-lights-tests
…om-entity-manager' into RJD-1057-traffic-lights-tests

Signed-off-by: Mateusz Palczuk <[email protected]>
Copy link

github-actions bot commented Nov 8, 2024

Checklist for reviewers ☑️

All references to "You" in the following text refer to the code reviewer.

  • Is this pull request written in a way that is easy to read from a third-party perspective?
  • Is there sufficient information (background, purpose, specification, algorithm description, list of disruptive changes, and migration guide) in the description of this pull request?
  • If this pull request contains a destructive change, does this pull request contain the migration guide?
  • Labels of this pull request are valid?
  • All unit tests/integration tests are included in this pull request? If you think adding test cases is unnecessary, please describe why and cross out this line.
  • The documentation for this pull request is enough? If you think adding documents for this pull request is unnecessary, please describe why and cross out this line.

@dmoszynski dmoszynski marked this pull request as ready for review November 10, 2024 14:27
@TauTheLepton TauTheLepton self-assigned this Nov 12, 2024
@dmoszynski dmoszynski marked this pull request as draft November 13, 2024 14:00
@TauTheLepton TauTheLepton added wait for regression test bump patch If this pull request merged, bump patch version of the scenario_simulator_v2 labels Nov 13, 2024
@TauTheLepton
Copy link
Contributor

No regressions confirmed here

@TauTheLepton TauTheLepton marked this pull request as ready for review November 14, 2024 17:34
@yamacir-kit yamacir-kit self-requested a review November 18, 2024 04:50
@TauTheLepton TauTheLepton marked this pull request as draft November 19, 2024 09:51
TauTheLepton and others added 10 commits November 19, 2024 11:19
Change 'random' numbers to round ones like one second or half a second

Signed-off-by: Mateusz Palczuk <[email protected]>
Add clear thresholds for tests with explanations and use these thresholds instead of hardcoded values

Signed-off-by: Mateusz Palczuk <[email protected]>
… instead of "awf/universe"

Signed-off-by: Mateusz Palczuk <[email protected]>
Make the variables const and add scope for subscribers so they stop listening (only in tests with more than one subscriber + remove .reset() as scopes take care of that

Signed-off-by: Mateusz Palczuk <[email protected]>
Copy link

sonarcloud bot commented Nov 25, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump patch If this pull request merged, bump patch version of the scenario_simulator_v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants