Review summary
- Requirements for code and reviews
- Previous code review issues index
- Follow style guide for Python Code (PEP 8), if the team does that have any other preferences
- Use uniform writing style (of variables, functions, classes, files)
- Use understandable naming (for variables, functions, classes, files)
- Use English
- Do not create separate code for completing tasks
- Do not duplicate code
- Do not use global variables
- Use pyrealsense2
- Use manual camera exposure and white balance
- Use wide camera resolutions and higher frame rates
- Align camera depth frames with color frames when using depth data
- Use serial.tools.list_ports (or equivalent) to find correct serial port
- Use enums
- Avoid unnamed/hardcoded/repeated constants
- Create configuration files for constants that need to be changed
- Use loops instead of repeating similar code
- Keep different parts of the code separate (separation of concerns)
- Use state machine for game logic
- Create separate functions for state handlers
- Make manual control one of the game logic states
- Do not use hardcoded movement functions
- Use proportional driving
- Use omni-motion to calculate motor speeds
- Do not open serial port connection multiple times
- Do not send commands to mainboard too frequently
- Close opened resources (camera, serial port, files)
- Fix typos
- Remove unused code
- Use WebSockets for receiving referee signals
- Do not send anything over WebSocket to robot basketball manager
- Make sure that all the functionality has been implemented that is required in the task description
- Test and review the code
- Do not overuse threads
- Use unpacking operator (in Python)
- Use map() or list comprehension (in Python)
- Use elif (else if)
- Use finally clause
- Throw and handle exceptions
- Create connect/open method for serial connection
- Sample multiple depth pixels to find less noisy distance
- Avoid driving too close to basket
- Other code style improvements
- Performance optimisation
- Other
Requirements for code and reviews
Created Merge Requests should be small in scope
To make Merge Requests as smooth as possible let’s keep one MR-s scope to one task.
If multiple tasks are combined into one MR it becomes hard to review due to the sheer scope of changes. This increases the time a review takes. Additionally, it is harder for the codes author to fix issues since systematic mistakes can be repeated more, creating more places needing fixing.
Follow PEP 8 coding conventions
Let’s follow PEP 8 coding conventions when writing code. They can be found here: https://peps.python.org/pep-0008/. This allows us to unify coding conventions and write better-to-read that can be understood faster by more people.
Use python logging instead of "print(…)" in final code
While printing stuff to console is a simple approach to get fast results it is hard to fine tune and configure later on without changes in code. This becomes a real issue when the project grows, and it is needed to enable or disable printing for debugging purposes or gaining the few percentage points of performance you need by disabling all logging during a competition.
print("Something happened!") # BAD
logger.info("Something happened!") # GOOD
TL;DR: Logging seems like a lot of effort but it simple to set up and allows for better configurability that improves code debugging and performance when utilizing the tools correctly.
Previous code review issues index
Use uniform writing style (of variables, functions, classes, files)
Use understandable naming (for variables, functions, classes, files)
Do not duplicate code
-
https://github.com/ut-robotics/picr21-team-sauna-madis/issues/25
-
https://github.com/ut-robotics/picr21-team-sauna-madis/issues/45
-
https://github.com/ut-robotics/picr21-team-tlo-rock/issues/23
-
https://github.com/ut-robotics/picr21-team-tlo-rock/issues/44
-
https://github.com/ut-robotics/picr21-team-tlo-rock/issues/45
Do not use global variables
Use wide camera resolutions and higher frame rates
Align camera depth frames with color frames when using depth data
Use serial.tools.list_ports (or equivalent) to find correct serial port
Keep different parts of the code separate (separation of concerns)
Use state machine for game logic
Other
-
https://github.com/ut-robotics/picr21-team-sauna-madis/issues/47
-
https://github.com/ut-robotics/picr21-team-tlo-rock/issues/68
-
https://github.com/ut-robotics/picr21-team-tlo-rock/issues/73
-
https://github.com/ut-robotics/picr21-team-robothree/issues/5
-
https://github.com/ut-robotics/picr21-team-robothree/issues/10