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

Improved reloading of OpenDRIVE maps in OSC #752

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Conversation

fabianoboril
Copy link
Collaborator

@fabianoboril fabianoboril commented Apr 15, 2021

Before enforcing a reload of the CARLA map when using OpenSCENARIO
the currently loaded map is compared with the new map. This comparison
was reworked to enforce that both maps start with the '' start tag:

  • Perform map string comparison only for content after tag
  • Load OpenDRIVE map using vertex_distance etc. parameters

Reworked handling of TrafficManager inside ScenarioRunner to improve
startup time:

  • Moved TrafficManager code from constructor to _load_and_run_scenario after updating the world

This change is Reviewable

Before enforcing a reload of the CARLA map when using OpenSCENARIO
the currently loaded map is compared with the new map. This comparison
was reworked to enforce that both maps start with the '<OpenDRIVE>' start tag.

Reworked handling of TrafficManager inside ScenarioRunner to improve
startup time.

Change-Id: I5d2fac8fcdc9a5bdd7fbb47589f3a1b535188175
@fabianoboril fabianoboril force-pushed the fix/reloadXODR branch 2 times, most recently from 0f09fc8 to cf53d71 Compare April 16, 2021 08:21
Copy link
Contributor

@glopezdiest glopezdiest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 2 files at r1, 2 of 2 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @fabianoboril and @fpasch)


scenario_runner.py, line 321 at r3 (raw file):

CarlaDataProvider.set_client(self.client)

Not really an issue but we can move this to the init, where the client is created


scenario_runner.py, line 364 at r3 (raw file):

Quoted 5 lines of code…
        CarlaDataProvider.set_traffic_manager_port(int(self._args.trafficManagerPort))
        tm = self.client.get_trafficmanager(int(self._args.trafficManagerPort))
        tm.set_random_device_seed(int(self._args.trafficManagerSeed))
        if self._args.sync:
            tm.set_synchronous_mode(True)

No comments here. However, I saw that we aren't setting the TM to async on cleanup, which we should do definitely do.

@fabianoboril
Copy link
Collaborator Author

Reviewed 1 of 2 files at r1, 2 of 2 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @fabianoboril and @fpasch)

scenario_runner.py, line 321 at r3 (raw file):

CarlaDataProvider.set_client(self.client)

Not really an issue but we can move this to the init, where the client is created

scenario_runner.py, line 364 at r3 (raw file):

Quoted 5 lines of code…

        CarlaDataProvider.set_traffic_manager_port(int(self._args.trafficManagerPort))
        tm = self.client.get_trafficmanager(int(self._args.trafficManagerPort))
        tm.set_random_device_seed(int(self._args.trafficManagerSeed))
        if self._args.sync:
            tm.set_synchronous_mode(True)

No comments here. However, I saw that we aren't setting the TM to async on cleanup, which we should do definitely do.

Done.

Copy link
Contributor

@glopezdiest glopezdiest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r4.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @fpasch)

@glopezdiest glopezdiest merged commit 23f0e53 into master Apr 16, 2021
@glopezdiest glopezdiest deleted the fix/reloadXODR branch April 16, 2021 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants