Skip to content

Conversation

npentrel
Copy link
Contributor

@npentrel npentrel commented Aug 11, 2023

Am I misunderstanding this or are we using sensor as a synonym

@npentrel npentrel requested a review from a team as a code owner August 11, 2023 09:51
@github-actions
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 is_moving
board gpio_pin_by_name
camera get_image
encoder get_position
motor is_moving
sensor get_readings
servo get_position
arm get_end_position
gantry get_lengths
gripper is_moving
movement_sensor get_linear_acceleration
input_controller get_controls
audio get_properties
pose_tracker get_poses
motion get_pose
vision get_classifications_from_camera

Copy link
Member

@njooma njooma left a comment

Choose a reason for hiding this comment

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

This is fine by me but this text was copied from RDK so we should change there as well cc @bazile-clyde

@bazile-clyde
Copy link

@npentrel I didn't write this but I suspect the reason the word "camera" wasn't used is because this method was implemented for the Intel Real Sense.

That is one camera with multiple "imagers" as they call it. Each takes a separate image and we use the timestamps to confirm both images were taken at roughly the same time.

To answer your question, no, "sensor" was not used synonymously with "camera". However, I'm not opposed to changing the word here, though, "camera" might be a bit overloaded since now it's referring to the imagers and the camera housing the imagers, the camera component.

@npentrel
Copy link
Contributor Author

should we use imgers then?

@bazile-clyde
Copy link

@npentrel imagers would be fine by me.

@njooma
Copy link
Member

njooma commented Aug 24, 2023

@npentrel has there been any movement on this? I fear it becoming stale...

@npentrel
Copy link
Contributor Author

let me find the pr on the rdk and ask there - and then I'll follow up here

@npentrel
Copy link
Contributor Author

Ok - @bhaney added the RDK code - what do you think Bijan - should we change this to imagers?

@bazile-clyde
Copy link

Ok - @bhaney added the RDK code - what do you think Bijan - should we change this to imagers?

@npentrel He's OOO for about two weeks, and my team owns this code. I'm okay with leaving it as is or changing it to "imagers."

@npentrel
Copy link
Contributor Author

ok let's do imagers then

@npentrel
Copy link
Contributor Author

viamrobotics/rdk#2833

@npentrel npentrel changed the title Rename sensor to camera Rename sensor to imager Aug 24, 2023
@njooma njooma self-requested a review August 24, 2023 18:14
@npentrel
Copy link
Contributor Author

@bazile-clyde ok to merge?

Copy link

@bazile-clyde bazile-clyde left a comment

Choose a reason for hiding this comment

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

LGTM!

@npentrel npentrel merged commit f65ba27 into main Aug 25, 2023
@npentrel npentrel deleted the getimages branch August 25, 2023 12:18
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.

4 participants