Skip to content

Conversation

martha-johnston
Copy link
Contributor

This change adds support for the new encoder API in the Python SDK. All of the tests in test_encoder.py pass successfully, however there is a consistent IDE error of a type mismatch whenever using the PositionType enum. It shouldn't be an actual issue (as it's not causing any tests to fail) but for reference, those errors occur at the following locations:

  • src/viam/components/encoder/service.py, line 49, request.position_type, and line 51, pos_type
  • tests/mocks/components.py, line 442, return ... self.position_type
  • examples/server/v1/components.py, line 454, return ... self.position_type

@martha-johnston martha-johnston requested a review from a team as a code owner April 11, 2023 20:57
@martha-johnston martha-johnston requested review from njooma, maximpertsov and randhid and removed request for njooma and maximpertsov April 11, 2023 20:57
Copy link
Contributor

@randhid randhid left a comment

Choose a reason for hiding this comment

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

Few nits.

Report the position of the encoder.
The value returned is the current position either in relative units (ticks away from a zero position)
or absolute units (degrees along a circle).

Copy link
Member

Choose a reason for hiding this comment

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

This should have a comment describing the parameter position_type and what it is/used for

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.

So the test is failing because evidently python 3.9 is being stricter with types. So everywhere you take PositionType as a param/return, you should make it PositionType.ValueType.

[nit] Also, be sure to organize your imports. You can do so in VS Code by pressing Option+Shift+O

@martha-johnston martha-johnston requested a review from njooma April 18, 2023 20:57
martha-johnston and others added 3 commits April 19, 2023 12:18
Co-authored-by: Naveed Jooma <naveedjooma@gmail.com>
Co-authored-by: Naveed Jooma <naveedjooma@gmail.com>
@martha-johnston martha-johnston requested a review from njooma April 19, 2023 16:26
@martha-johnston martha-johnston merged commit a9a3f57 into viamrobotics:main Apr 19, 2023
@martha-johnston martha-johnston deleted the rsdk-2483 branch April 19, 2023 21:23
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