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.

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

GPS and Drone Modules #17

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

GPS and Drone Modules #17

wants to merge 10 commits into from

Conversation

ArjunGupte1
Copy link
Collaborator

Added all new files and modifications to old files for the GPS and drone modules (on the Python side).

Copy link
Contributor

@MatthewCalligaro MatthewCalligaro left a comment

Choose a reason for hiding this comment

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

Good job! Most of my feedback is in coordination with MITLLRacecar/Simulation#1. Let's discuss and check these in together when they are ready!

library/drone.py Outdated
"""
Copyright MIT and Harvey Mudd College
MIT License
Summer 2020
Copy link
Contributor

Choose a reason for hiding this comment

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

Go ahead and make that 2022 :)

library/drone.py Outdated
# Store the amount of blue in the pixel on row 3, column 5
blue = image[3][5][0]
"""
return copy.deepcopy(self.get_drone_image_no_copy())
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a declaration for get_image_no_copy as well (see camera.py for comparison).

library/drone.py Outdated
_HEIGHT: int = 480


def get_drone_image(self) -> NDArray[(480, 640, 3), np.uint8]:
Copy link
Contributor

Choose a reason for hiding this comment

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

To keep in line with my comment in the Simulation PR review, consider changing this to get_image instead. Happy to discuss if you disagree.

library/drone.py Outdated
pass

@abc.abstractmethod
def get_drone_height(self) -> float:
Copy link
Contributor

Choose a reason for hiding this comment

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

consider changing to get_height

library/drone.py Outdated
@abc.abstractmethod
def get_drone_height(self) -> float:
"""
Returns the drone's height.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add an Example:: and Returns:: entry to the docstring. This is needed so that the documentation is auto-formatted correctly on the website (the website pulls from these docstrings to make the html).

self.__drone_image: NDArray[(480, 640, 3), np.uint8] = None
self.__is_drone_image_current: bool = False

def get_drone_image_no_copy(self) -> NDArray[(480, 640, 3), np.uint8]:
Copy link
Contributor

Choose a reason for hiding this comment

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

any function names changed in drone.py will have to be made here as well

def get_drone_image_async(self) -> NDArray[(480, 640, 3), np.uint8]:
return self.__request_drone_image(True)

def __update(self) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Organize by public and private methods (ie move __update and __request_drone_image to the bottom)

@@ -20,6 +20,7 @@
import drive_sim
import lidar_sim
import physics_sim
import drone_sim
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to change __VERSION = 1 to 2 since you are updating the communication protocol.

@@ -65,6 +66,11 @@ class Header(IntEnum):
lidar_get_samples = 26
physics_get_linear_acceleration = 27
physics_get_angular_velocity = 28
physics_get_position = 29
Copy link
Contributor

Choose a reason for hiding this comment

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

Any changes in Simulation/PythonInterface.cs will need to be made here too

@@ -263,3 +271,4 @@ def __handle_error(self, error: Error):
rc_utils.print_error(text)
print(">> Closing script...")
exit(0)

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove extra newline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants