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-5936: change return type for get readings #506

Conversation

purplenicole730
Copy link
Member

@purplenicole730 purplenicole730 commented Dec 21, 2023

The return type for the get_readings() calls should be of Mapping[str, viam.utils.ValueTypes] rather than Mapping[str, Any].

Tested with a fake movement sensor and power sensor.

fakeMovementSensor get_readings return value:
{'linear_acceleration': x: 2.2
y: 4.5
z: 2
, 'angular_velocity': z: 1
, 'compass': 25.0, 'position': latitude: 40.7
longitude: -73.98
, 'linear_velocity': y: 5.4
, 'orientation': {'r': 1.0, 'i': 0.0, '_type': 'quat', 'k': 0.0, 'j': 0.0}, 'altitude': 50.5}
fake get_readings return value:
{'volts': 1.5, 'watts': 9.8, 'is_ac': True, 'amps': 2.2}

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
power_sensor get_power
motion get_pose
vision get_classifications_from_camera

@purplenicole730 purplenicole730 marked this pull request as ready for review December 22, 2023 02:44
@purplenicole730 purplenicole730 requested a review from a team as a code owner December 22, 2023 02:44
@purplenicole730 purplenicole730 requested review from njooma, stuqdog and martha-johnston and removed request for njooma and stuqdog December 22, 2023 02:44
Copy link
Contributor

@martha-johnston martha-johnston left a comment

Choose a reason for hiding this comment

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

lgtm assuming you have tested with a real (or fake) sensor not just the python tests. just want to make sure changing the type didn't mess up something somewhere else

src/viam/utils.py Outdated Show resolved Hide resolved
@@ -172,7 +175,7 @@ async def get_geometries(
return [geometry for geometry in response.geometries]


def sensor_readings_native_to_value(readings: Mapping[str, Any]) -> Mapping[str, Any]:
def sensor_readings_native_to_value(readings: Mapping[str, Any]) -> Mapping[str, Value]:
Copy link
Member

Choose a reason for hiding this comment

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

Good catch

@purplenicole730 purplenicole730 merged commit 63c03d8 into viamrobotics:main Dec 27, 2023
12 checks passed
@purplenicole730 purplenicole730 deleted the RSDK-5936-change-return-type-for-get_readings branch December 27, 2023 16:58
nicksanford pushed a commit to nicksanford/viam-python-sdk that referenced this pull request Jan 29, 2024
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.

3 participants