AutoMecUA / AutoMec-AD

Autonomous RC car with the help of ROS Noetic and ML.
GNU General Public License v3.0
15 stars 2 forks source link

Clean code #143

Closed manuelgitgomes closed 1 year ago

manuelgitgomes commented 2 years ago

Due to last-minute changes and bad coding practices, code cleansing needs to happen.

brnaguiar commented 2 years ago

datasets:

modelos (.h5):

manuelgitgomes commented 2 years ago

As seen in other repos such as Major Alvega's and AtlasCar2's, a refactoring of our packages is needed to reduce the number of packages and the complexity. I suggest doing as follows:

In the future, we can add other packages such as prometheus_vertical_signals, prometheus_parking, etc....

Examples shown here: MicrosoftTeams-image (12) image

brnaguiar commented 2 years ago

YAML file structures:

model:
    name: "..."
    developer: $automec_dev
    date: 25-01-2022 12h40
    ml_arch: 
        name: 'rota'
        epochs: 20
        batch_size: 30
        val_loss:
        test_loss:
    dataset: 
        name: "..."
        cam_pose: "..."
        developer: $automec_dev
        environment: ema
        frequency: 30
        image_size: 320,160
        image_number: 32650
        linear_velocity: 0
        type: parking/driving/etc
        date:  25-01-2022 12h20
        comments: "..."
    model_eval: 5*
    comments: "..."
dataset:
  name: "..."
  cam_pose: "..."
  developer: $automec_dev
  environment: ema
  frequency: 30
  image_size: 320,160
  image_number: 32650
  linear_velocity: 0
  type: parking/driving/etc
  date:  25-01-2022 12h20
  comments: "..."
manuelgitgomes commented 2 years ago

Prometheus gazebo done. Now for the prometheus_description, I think we should take the change and fix the urdf once and for all.

Almeida-a commented 2 years ago

Signal Recognition The code itself looks good. I have only carried out some detailed tweaks. On a second task, we/I can also review the concepts of this module and reenforce the documentation, if you think is appropriate.

Also, I suggest we adding type hinting for better readability for the functions' signature, like the next example:

   def fun(value: int, matrix: numpy.ndarray) -> str:
      pass
manuelgitgomes commented 2 years ago

Further changes for yaml:

Further suggestions:

manuelgitgomes commented 2 years ago

Further changes in prometheus_gazebo:

Further suggestions:

manuelgitgomes commented 2 years ago

Also, I suggest we adding type hinting for better readability for the functions' signature, like the next example:

We can also try docstrings! We can also add them for scripts. https://docs.python.org/3/library/doctest.html#module-doctest https://peps.python.org/pep-0257/

manuelgitgomes commented 2 years ago

We should use:

manuelgitgomes commented 2 years ago

We should replace all global variables using partials and iterable variables. Example: https://github.com/lardemua/atom/blob/c3605c6c370eac286bc1d4163095d6e6685ae0e2/atom_calibration/scripts/dataset_playback#L213-L216

https://github.com/lardemua/atom/blob/c3605c6c370eac286bc1d4163095d6e6685ae0e2/atom_calibration/src/atom_calibration/dataset_playback/depth_manual_labeling.py#L39-L81

manuelgitgomes commented 2 years ago

Use PEP-8.

Study walrus operator:

https://www.geeksforgeeks.org/walrus-operator-in-python-3-8/

sample_data = [
    {"userId": 1, "name": "rahul", "completed": False},
    {"userId": 1, "name": "rohit", "completed": False},
    {"userId": 1, "name": "ram", "completed": False},
    {"userId": 1, "name": "ravan", "completed": True}
]

print("With Python 3.8 Walrus Operator:")
for entry in sample_data:
    if name := entry.get("name"):
        print(f'Found name: "{name}"')

print("Without Walrus operator:")
for entry in sample_data:
    name = entry.get("name")
    if name:
        print(f'Found name: "{name}"')
manuelgitgomes commented 2 years ago

Simulation is basically done. Sensors need to be added, meshes need to be changed and measures need to be taken. But our boy is here: image

brnaguiar commented 2 years ago

TASKS DONE:

NEEDS TO BE DONE:

manuelgitgomes commented 2 years ago

Change signals from 0º to 25º

brnaguiar commented 2 years ago

bug: image capture starts automatically when running data acquisition command.

brnaguiar commented 2 years ago
  • add comment input

Done

bug: image capture starts automatically when running data acquisition command.

Fixed: it wasn't really a bug, it behaves like it should: only recording when in movement.

I had an additional idea: sometimes, when performing the data acquisition task, we don't want to save the datasets due to some error, for example. In that case, I think we should implement an option to delete the created folder for the dataset and IMGs, using for example the keys: q[uit] and s[ave], to discard the dataset and to save the dataset.

manuelgitgomes commented 2 years ago

@Almeida-a try using a config dictionary for begin image, mask_mode, other config variables...

manuelgitgomes commented 2 years ago

@Almeida-a This needs cleaning as well! https://github.com/AutoMecUA/AutoMec-AD/blob/manuelgitgomes/issue143/cnn/scripts/write.py

manuelgitgomes commented 2 years ago

Sensors added!

Almeida-a commented 2 years ago

Made some clean-up here

manuelgitgomes commented 2 years ago

Hello! Simulation is totally functional, using the command:

roslaunch prometheus_gazebo arena.launch

to launch gazebo, and the command

roslaunch prometheus_bringup bringup.launch sim:=true

to bringup prometheus inside gazebo. This launch file is also intended to be used in a real environment, changing the sim argument. Another argument can be used:

roslaunch prometheus_bringup bringup.launch sim:=true visualization:=true

where an rviz window displays the sensor data.

The bringup of the real robot is not yet implemented, but work has started on it.

A caveat, as the models changed folders, the user need to input this to their .bashrc/.zshrc:

export GAZEBO_MODEL_PATH="`rospack find prometheus_gazebo`/models:${GAZEBO_MODEL_PATH}" 
Almeida-a commented 2 years ago

Task: delete the unpacking tokens (**) that I introduced in this commit.

Almeida-a commented 2 years ago

Task: delete the unpacking tokens (**) that I introduced in this commit.

Done.

Next steps are to test out the code. After that, we can merge dev branch onto manuelgitgomes/issue143 and test that version out.

manuelgitgomes commented 2 years ago

Updates! Kinects are fully functional and compatibility between environments is achieved.

Dataset writing is also functional in simulation. Few tweaks need to be made to clean code and enhance compatibility with the real environment. One of these tweaks is the removal of androind_inputs to favor an automatically capped twist message (/cmd_vel).

Some "core" things were changed for compatibility purposes, but they can be revoked if chosen. cam_pose is removed from the yaml file in favor of only the urdf. If after every change the robot is calibrated, a new urdf is generated and the cam_pose is deprecated information. If there is no time to calibrate the robot, these changes can be stated on the comments section,

Known issues: Simulation's pointclouds are dislocated, cannot find a way to straighten them. As refered in #155, pandas gives the following warning:

FutureWarning: The frame.append method is deprecated and will be removed from pandas in a future version. Use pandas.concat instead.
  driving_log = driving_log.append(row, ignore_index=True)
Almeida-a commented 2 years ago

As refered in #155, pandas gives the following warning:

FutureWarning: The frame.append method is deprecated and will be removed from pandas in a future version. Use pandas.concat instead.
  driving_log = driving_log.append(row, ignore_index=True)

Follow this example to fix it:

# Append new row to dataframe "stats"
row = pd.DataFrame(
    dict(filename=[outfile_name], cs=[cs], ds=[ds], cr=[cr], mse=[mse], psnr=[psnr], ssim=[ssim])
)
stats = pd.concat([stats, row])
manuelgitgomes commented 2 years ago

Hello!

Few tweaks need to be made to clean code and enhance compatibility with the real environment. One of these tweaks is the removal of androind_inputs to favor an automatically capped twist message (/cmd_vel).

This is now done, using the twist_cap.py script, in the prometheus_bringup package. Controller support is added with joy_teleop.py in the prometheus_bringup package. Some changes to the jupyter file were added, in order to automatize the model naming - @brnaguiar can you check if everything is good to you? The changes were in block 68.

manuelgitgomes commented 2 years ago

Changes signal recognition to be functional in this new system. One large change, the signal recognition node now publishes the signal detected to the topic, leaving the decision to the driving node.. Further changes for this recognition:

manuelgitgomes commented 2 years ago
  • Changing the sequential detection. The way it is, the system only defines a detection if detected on the last two frame. It can be more robust, such as in 4 of the last 5 frames

This is now implemented.

Autonomous driving in now reformulated. It reads the signal topic and acts accordingly (goes forward when 'pForward', stops when 'pStop', stops and end the program when 'pChess'). At the end the user is prompted to add comments and evaluate.

With this, the reformation of the code is over. After testing with the real car, documentation is needed and then a merge to the main branch accompanied with release is my suggestion. The release can follow something as @Almeida-a has done: https://github.com/Almeida-a/imodec-dicoogle-plugin/releases/tag/v0.1.0-a0

manuelgitgomes commented 2 years ago

Created Users' Guide to Hardware, for every hardware related issue. Needs pictures of real car to complement the instructions. Created Users' Guide to Software, for every software related issue. It now present every pre requisite needed to use our code. The description and commands for our code is missing yet.

PedroMS3 commented 1 year ago

Everything should be done regarding this issue