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

[RSDK-9520] Refactor NextPointCloud -> PointCloud and add extra param #4628

Closed
wants to merge 4 commits into from

Conversation

hexbabe
Copy link
Member

@hexbabe hexbabe commented Dec 13, 2024

Refactor camera interface: rename NextPointCloud to PointCloud and update related tests

This PR updates the camera interface by renaming the method NextPointCloud to PointCloud, reflecting a more consistent naming convention. All instances of the method across various files, including tests and implementations, have been modified accordingly. The new method signature includes the extra parameter to match the proto API, enhancing flexibility and aligning the interfaces.

Changes include:

  • Updated method definitions and calls in camera.go, client.go, videosourcewrappers.go, and other related files.
  • Adjusted test cases to use the new method name and signature.

This refactor improves code clarity and prepares the codebase for further updates to point cloud processing.

…date related tests

This commit updates the camera interface by renaming the method `NextPointCloud` to `PointCloud`, reflecting a more consistent naming convention. All instances of the method across various files, including tests and implementations, have been modified accordingly. The new method signature includes the `extra` parameter to match the proto API, enhancing flexibility and aligning the interfaces.

Changes include:
- Updated method definitions and calls in `camera.go`, `client.go`, `videosourcewrappers.go`, and other related files.
- Adjusted test cases to use the new method name and signature.

This refactor improves code clarity and prepares the codebase for further updates to point cloud processing.
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Dec 13, 2024
Copy link
Contributor

Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!

component function
base IsMoving
board GPIOPinByName
camera Properties
encoder Properties
motor IsMoving
sensor Readings
servo Position
arm EndPosition
audio MediaProperties
gantry Lengths
gripper IsMoving
input_controller Controls
movement_sensor LinearAcceleration
power_sensor Power
pose_tracker Poses
motion GetPose
vision GetProperties

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 13, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 13, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 13, 2024
@hexbabe hexbabe requested review from randhid and seanavery December 13, 2024 18:35
// a part of a sequence. In the future, there could be streaming of point clouds.
NextPointCloud(ctx context.Context) (pointcloud.PointCloud, error)
PointCloud(ctx context.Context, extra map[string]interface{}) (pointcloud.PointCloud, error)
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

no.

Copy link
Member Author

Choose a reason for hiding this comment

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

noooooooooo

Copy link
Member

Choose a reason for hiding this comment

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

We are ok to handle mimetype inclusion in another PR? There are no consequences wrt data capture by adding in extra to the interface method here?

Copy link
Member

Choose a reason for hiding this comment

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

We are ok to handle mimetype inclusion in another PR?

If we are going to break the same interface I'd rather not do it twice.

@hexbabe hexbabe closed this Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants