Fixes and new features

[johnMinelli]

Hi, I listed the changes here. Please review them carefully as said in (#67) i hope i'm not breaking anything with previous CARLA versions.

UPDATE

• CAMERA_TYPES dictionary with supported camera types inside CameraManager
• possibility of use different camera types with also different pov by using in the config 'rgb', 'rgb-0', rgb-1'. Note: on missing position specification the default one is used
• code fixed for depth camera use
• possibility of specification of multiple scenarios in the config file in list format

BREAKING CHANGES

• I changed the scenarios file to be an "abstract class" extensible (by override) with additional scenarios
• _encode_obs fuinction now take the original image as argument and a decode_obs is introduced as well. In this way when extending the environment one can apply the proper encodng/decoding logic, getting the raw data and modelling the observation for proper needs.
• _spawn_new_agent function become _spawn_new_actor for naming convention compliance
• _get_free_tcp_port function become _get_tcp_port adding the port as parameter
• set_senor function header changed with the ability of specify the aditional parameter (shouldn't be so problematic being the parameter setted with a default value)

MINORS CHANGES

in nodeid_coord_map.py:

• MAP_TO_COORDS_MAPPING dict to address directly the node ids dictionary

in muti-env.py:

• the attribute _dones was removed to simplify the code
• The inizialization of the class Rewards has been moved in the init to be overridable
• _image_space fixed respect the current preprocessing done with preprocess_image function used
• some changes of structure in the _reset and reset functions to do a proper reset
• dones in step function computed before rewards to provide that additional field later in py_measurements variable

in keyboard_controller.py:

• small bug fix

in hud.py:

• notification disabled (need some work if there is such will of implementation)

[lgtm-com]

This pull request introduces 1 alert when merging 62e0b3c into bf40a53 - view on LGTM.com

new alerts:

• 1 for Unused local variable

[praveen-palanisamy]

Hi @johnMinelli ,
Thank you for this PR and a detailed summary of the updates and changes. I appreciate your efforts to make this library much better.
I started reviewing this and realized there is a missing module in this PR branch. I left a comment previously to help address this by breaking down the changes into independent but self-contained PRs.

[johnMinelli]

Sorry I didn't understand, you are suggesting of splitting the PR in many ones? The missing module is missing from master or i've done a mistake and the code is referencing to something that is not in this PR?

[praveen-palanisamy]

No worries. Let me try to clarify:
The missing module was referring to this previous review comment I left on this PR, reproduced below as an image for a quick reference:

I believe, you see the above comment on this page.

Splitting the PR was a suggestion to keep each PR self-contained and targeted towards a particular bug-fix/feature/enhancement. This was a suggestion going forward.

Hope that helps?

EDIT: Oops. The issue got closed by some mistaken key press combination. I have re-opened it.

[johnMinelli]

Yes, you are right. Anyway I don't have any comment on the code, I don't know why I cannot see that

[praveen-palanisamy]

@johnMinelli : With #70 now merged to the main branch, please resolve the conflicts on this PR to make clear the new contributions in this PR.
Thank you!

[lgtm-com]

This pull request introduces 1 alert and fixes 1 when merging 9425e11 into 24907d8 - view on LGTM.com

new alerts:

• 1 for Syntax error

fixed alerts:

• 1 for Binding a socket to all network interfaces

---

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

[praveen-palanisamy]

The macad_gym.core.controllers.traffic module doesn't exist in this PR. I see that you have defined that module in your dev_traffic branch and have created another #70 .

It will be better if you can break down the changes into separate but complete PRs.

[johnMinelli]

do you still confirm that I should remove the apply_traffic and npc code, since the other branch has already been merged?

[johnMinelli]

Let me know for the other changes if I should do something or set them as resolved if you agree