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

fix: wedo2 stop motor when power set to 0 #198

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

tpsnt
Copy link

@tpsnt tpsnt commented Feb 28, 2024

When wedo2 motor power set to 0, the motor stops.

Resolves

When wedo2 motor power set to 0, the motor won't stop. And motorOff() won't stop the motors either.

Proposed Changes

This change adds a judge statement. When a startMotorPower() is invoked, it checks args.POWER. If args.POWER === 0, call a motor.turnOff first, then change the power attribute of the motor.

Reason for Changes

In WeDo2Motor.turnOff(), if this._power === 0, it returns immediately.
In Scratch3WeDo2Blocks.startMotorPower(), if args.POWER === 0, WeDo2Motor.turnOn() returns immediately.
So when you call Scratch3WeDo2Blocks.startMotorPower({POWER:0}), you won't turnOff() the motor, until you set the power to a number other and 0.

Test Coverage

Scratch3WeDo2Blocks.startMotorPower({POWER:0});
WeDo2Motor.turnOff();

@GarboMuffin
Copy link
Member

  • can you make it an if/else so that you're only calling either turnOn or turnOff, not both (yes I know the turnOn will do nothing, but that is still weird code)
  • is Scratch aware of this / is there a PR open up there?

@tpsnt
Copy link
Author

tpsnt commented Feb 29, 2024

  • can you make it an if/else so that you're only calling either turnOn or turnOff, not both (yes I know the turnOn will do nothing, but that is still weird code)
  • is Scratch aware of this / is there a PR open up there?

scratchfoundation#4207
but scratchfoundation doesn't reply to me, and there isn't open issues there.

@GarboMuffin GarboMuffin merged commit b6c4208 into TurboWarp:develop Feb 29, 2024
1 check passed
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.

2 participants