-
Notifications
You must be signed in to change notification settings - Fork 190
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
Remove vehicle type from agent interface #1294
base: develop
Are you sure you want to change the base?
Conversation
if mission.vehicle_spec == None: | ||
vehicle_type = "sedan" | ||
else: | ||
vehicle_type = mission.vehicle_spec.veh_config_type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this the right place to get the vehicle type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, although I am slightly confused as to why the vehicle_spec
is inside the mission rather than in the plan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where should vehicle_spec
be specified then?
traffic[v.vehicle_id] = envision_types.TrafficActorState( | ||
name=self._agent_manager.agent_name(agent_id), | ||
actor_type=actor_type, | ||
vehicle_type=envision_types.VehicleType.Car, | ||
vehicle_type=veh_type, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems to cause the ego vehicle to be the wrong color (grey instead of red) on SUMO maps
"""Vehicle specifications""" | ||
|
||
veh_id: str | ||
veh_config_type: str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is problematic because we have been and are still distinguishing in our (core) code between veh_type
and veh_config_type
, where the latter should eventually be fully encapsulated to just the Sumo types (per Issue #1270). I do not think we should be exposing the "config" (Sumo) types in Scenario Studio, but to just change the variable name here to veh_type
would also be a misnomer without further unification in the core. Thus, I suggest we deal with this as part of the more comprehensive solution that issue #1270 requires.
#753