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
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions components/motor/motor.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,22 @@ func GetRequestedDirection(rpm, revolutions float64) float64 {
}
return dir
}

// GetSign returns the sign of the float as a helper for getting
// the intended direction of travel of a motor.
func GetSign(x float64) float64 {
if x == 0 {
return 0
}
if math.Signbit(x) {
return -1.0
}
return 1.0
}

// ClampPower clamps a percentage power to 1.0 or -1.0.
func ClampPower(pwr float64) float64 {
pwr = math.Min(pwr, 1.0)
pwr = math.Max(pwr, -1.0)
return pwr
}
22 changes: 18 additions & 4 deletions components/motor/ulnstepper/28byj-48.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,13 @@ func (m *uln28byj) GoFor(ctx context.Context, rpm, revolutions float64, extra ma
m.logger.CWarn(ctx, warning)
if err != nil {
m.logger.CError(ctx, err)
// only stop if we receive a zero RPM error
return m.Stop(ctx, extra)
}
return m.Stop(ctx, extra)
// we do not get the zeroRPM error, but still want
// the motor to move at the maximum rpm
m.logger.CWarnf(ctx, "can only move at maxRPM of %v", maxRPM)
rpm = maxRPM * motor.GetSign(rpm)
}

targetStepPosition, stepperDelay := m.goMath(rpm, revolutions)
Expand Down Expand Up @@ -372,12 +377,15 @@ func (m *uln28byj) SetRPM(ctx context.Context, rpm float64, extra map[string]int

// Set the current position (+/- offset) to be the new zero (home) position.
func (m *uln28byj) ResetZeroPosition(ctx context.Context, offset float64, extra map[string]interface{}) error {
newPosition := int64(-1 * offset * float64(m.ticksPerRotation))
if err := m.Stop(ctx, extra); err != nil {
return err
}
m.lock.Lock()
defer m.lock.Unlock()
m.stepPosition = int64(-1 * offset * float64(m.ticksPerRotation))
m.stepPosition = newPosition
m.targetStepPosition = newPosition
// use Stop to set the target position to the current position again
randhid marked this conversation as resolved.
Show resolved Hide resolved
return nil
}

Expand All @@ -391,13 +399,16 @@ func (m *uln28byj) SetPower(ctx context.Context, powerPct float64, extra map[str
m.logger.CWarn(ctx, warning)
if err != nil {
m.logger.CError(ctx, err)
// only stop if we receive a zero RPM error
return m.Stop(ctx, extra)
}
return m.Stop(ctx, extra)
}

m.lock.Lock()
defer m.lock.Unlock()
m.targetStepPosition = int64(math.Inf(int(powerPct)))
direction := motor.GetSign(powerPct) // get the direction to set target to -ve/+ve Inf
m.targetStepPosition = int64(math.Inf(int(direction)))
randhid marked this conversation as resolved.
Show resolved Hide resolved
powerPct = motor.ClampPower(powerPct) // ensure 1.0 max and -1.0 min
m.stepperDelay = m.calcStepperDelay(powerPct * maxRPM)

m.doRun()
Expand Down Expand Up @@ -432,6 +443,9 @@ func (m *uln28byj) Stop(ctx context.Context, extra map[string]interface{}) error
if m.doRunDone != nil {
m.doRunDone()
}
m.lock.Lock()
defer m.lock.Unlock()
m.targetStepPosition = m.stepPosition
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion components/motor/ulnstepper/28byj-48_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.

test.That(t, fmt.Sprint(latestLoggedEntry), test.ShouldContainSubstring, "nearly the max")
})

Expand Down
Loading