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-8486] Fix 28byj-48 control issues #4442

Merged
merged 2 commits into from
Oct 10, 2024

Conversation

randhid
Copy link
Member

@randhid randhid commented Oct 10, 2024

Addressed in this PR:

  • allow SetPower Calls to move backwards.
  • allow SetRPM calls with rpms above the max rpm to move with the maxrpm, and add a warning saying we're moving at the maxrpm
  • isMoving returns a correct state of the motor false when stopped, true when moving
  • ResetZero position does not falsely report that the motor is moving after resetting the position.

Manual testing:

  • control page execution of SetPower +ve/-ve, observed motor going forwards when expected and backwards when expected. 0 power did not move the motor.
  • control page execution of SetRPM +ve/-ve, observed motor going forward when expected. 0 rpm did not move the motor, magnitudes of rpm > maxrpm did move the motor with the added warning.
  • control page GoFor with combo of -ve/+ve rpm and -ve and +ve revolutions. Observed motor motion going forwards when expected and backwards when expected.
  • control page execution of GoTo moved the motor to the desired position, with -ve or +ve rpm.
  • SDK code run, GoFor, GoTo blocks, SetPower does not.
  • SDK code ResetZeroPosition stops a SetPower call and resets the position as well as setting is moving to false and returns 0 power, false on from IsPowered.
  • No hangs in server shutdown from SetPower interrupting GoFor or IsPowered run from a client script while a SetPower call is running.

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Oct 10, 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

@@ -268,7 +268,7 @@ func TestFunctions(t *testing.T) {
err = m.GoFor(ctx, 146, 1, nil)
test.That(t, err, test.ShouldBeNil)
allObs = obs.All()
latestLoggedEntry = allObs[len(allObs)-1]
latestLoggedEntry = allObs[len(allObs)-2]
Copy link
Contributor

Choose a reason for hiding this comment

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

what caused this to change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a new logger line to tell the user that we're setting the rpm to max, not stopping, when we're passing in an rpm larger than the maxRPM, it was stopping because the logic previously called a stop when any warning was returned from CheckSpeed, not we only stop when an error is returned - the zero rpm error.

Copy link
Member Author

Choose a reason for hiding this comment

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

So now we check the entry before last to make sure it's the maxrpm warning that we expect.

@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 Oct 10, 2024
@randhid randhid merged commit a381f05 into viamrobotics:main Oct 10, 2024
19 checks passed
@randhid randhid deleted the RSDK-8486-28-byj-48-control-issues branch October 18, 2024 14:27
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.

3 participants