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.

Sign up for GitHub

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

Jump to bottom

Npc traffic spawning feature #70

Merged
merged 8 commits into from
Nov 26, 2022
Merged

Npc traffic spawning feature #70

merged 8 commits into from
Nov 26, 2022

Conversation

johnMinelli
Copy link
Contributor

The scenarios can be customized adding vehicles and pedestrians keys in order to spawn n NPC in the map
Additionally the environment now present a public variable exclude_hard_vehicles which allow to exclude the big vehicles (note also that the fixed camera position clip in some of those vehicles)

@praveen-palanisamy
Copy link
Owner

@johnMinelli ,
Could you please respond to and/or resolve the review comments so that we can make progress towards merging this PR?
The review comments are listed on this page above.
Based on your comment here, it may be the case that the comments are not showing up on your end. If you are unable to see them on this page too for some reason, please share a screenshot of this page including this comment in order for me to understand your view/interface and provide you relevant information in an alternative way.

@johnMinelli
Copy link
Contributor Author

Hi, I would be happy to do it but I don't see any comment 🙂
image

praveen-palanisamy
praveen-palanisamy requested changes Nov 13, 2022
View reviewed changes
Copy link
Owner

@praveen-palanisamy praveen-palanisamy left a comment

Choose a reason for hiding this comment

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

Thank you for letting me know.
I am (re-)submitting partial review comments. Please address them to take this forward.
Please let me know if this feedback is also not visible to you.

src/macad_gym/carla/config.json Outdated Show resolved Hide resolved
src/macad_gym/carla/config.json Outdated
@@ -45,7 +45,7 @@
"reward_function": "corl2017",
"manual_control": false,
"auto_control": false,
"camera_type": "rgb",
"camera_type": "ray",
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please explain the rationale behind this change?
ray is not a camera_type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i added the ability to use many different camera types

Copy link
Owner

Choose a reason for hiding this comment

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

Okay but, what does the ray camera type refer to?
CARLA doesn't have a ray camera.

src/macad_gym/core/sensors/camera_manager.py Show resolved Hide resolved
src/macad_gym/core/sensors/camera_manager.py Outdated Show resolved Hide resolved
src/macad_gym/core/controllers/traffic.py Outdated Show resolved Hide resolved
src/macad_gym/core/controllers/traffic.py Show resolved Hide resolved
@johnMinelli
added camera_position config parameter; change of CAMERA_TYPES in E…
13a3054 
…num; small changes for PR comments resolution
johnMinelli
johnMinelli commented Nov 19, 2022
View reviewed changes
Copy link
Contributor Author

@johnMinelli johnMinelli left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for the delay. I made the changes as you suggested, let me know if it is ok.

praveen-palanisamy
praveen-palanisamy requested changes Nov 20, 2022
View reviewed changes
Copy link
Owner

@praveen-palanisamy praveen-palanisamy left a comment

Choose a reason for hiding this comment

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

@johnMinelli , Thank you for the updates! I have left a few minor comments to be resolved before we merge this in.

src/macad_gym/carla/config.json Outdated
@@ -45,7 +45,7 @@
"reward_function": "corl2017",
"manual_control": false,
"auto_control": false,
"camera_type": "rgb",
"camera_type": "ray",
Copy link
Owner

Choose a reason for hiding this comment

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

Okay but, what does the ray camera type refer to?
CARLA doesn't have a ray camera.

src/macad_gym/carla/multi_env.py Outdated Show resolved Hide resolved
src/macad_gym/carla/multi_env.py Show resolved Hide resolved
src/macad_gym/carla/multi_env.py Outdated Show resolved Hide resolved
@praveen-palanisamy
Copy link
Owner

@johnMinelli : Could you please resolve the comment above on the ray camera type?
If you add an example or at least a test case for that, it will be good for reference.
We can merge this PR once the review comments are resolved.
Thank you for the contributions!

@johnMinelli
Copy link
Contributor Author

oh sorry, I totally missed your reply

@praveen-palanisamy praveen-palanisamy merged commit 24907d8 into praveen-palanisamy:master Nov 26, 2022
@praveen-palanisamy praveen-palanisamy mentioned this pull request Nov 26, 2022
Closed
@johnMinelli johnMinelli deleted the dev_traffic branch May 20, 2023 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@praveen-palanisamy praveen-palanisamy praveen-palanisamy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@johnMinelli @praveen-palanisamy