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

Add relative_direction function to Pose #166

Merged
merged 3 commits into from
Aug 8, 2024
Merged

Conversation

pascalzauberzeug
Copy link
Contributor

@pascalzauberzeug pascalzauberzeug commented Aug 8, 2024

I wanted to know how much the robot has to turn to face a target.
You first have to get the direction to the target and then calculate the difference between the direction and your robot's current yaw.
This PR combines this approach in the Pose.relative_direction(Point | Pose) function to make it more easily available.

@falkoschindler the angle parameter of rotate(self, angle: float) collides with rosys.helpers.angle. I imported it as helpers_angle for now, but I don't really like it.

@pascalzauberzeug pascalzauberzeug added the enhancement New feature or request label Aug 8, 2024
@pascalzauberzeug pascalzauberzeug added this to the 0.14.0 milestone Aug 8, 2024
@pascalzauberzeug pascalzauberzeug self-assigned this Aug 8, 2024
@pascalzauberzeug pascalzauberzeug marked this pull request as draft August 8, 2024 17:28
@pascalzauberzeug pascalzauberzeug marked this pull request as ready for review August 8, 2024 17:37
Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Thanks, @pascalzauberzeug!

I changed the import to from .. import helpers to avoid conflicts with angle without the need for an alias.

When reading the code, I wonder why we use @overload for distance(), direction() and relative_direction(). It might be useful in case where, e.g., the return type depends on the input type like here:

@overload
def project_to_image(self, world_coordinates: Point3d) -> Point | None: ...
@overload
def project_to_image(self, world_coordinates: np.ndarray) -> np.ndarray: ...
def project_to_image(self, world_coordinates: Point3d | np.ndarray) -> Point | None | np.ndarray:

or where we want to use a different docstring for each variant. But distance() always returns float and has no docstring, so I don't see a benefit over a simply union type annotation. Of course, this isn't really the point of this PR. It's just a related thought we could address here or in a subsequent PR.

@falkoschindler falkoschindler merged commit e084bfd into main Aug 8, 2024
4 checks passed
@falkoschindler falkoschindler deleted the relative_direction branch August 8, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants