microbit-foundation / micropython-microbit-v2

Temporary home for MicroPython for micro:bit v2 as we stablise it before pushing upstream
MIT License
43 stars 24 forks source link

sonar sensor problem(hcsr04) - time_pulse_us #26

Closed crouse12 closed 3 years ago

crouse12 commented 3 years ago

The following code for hcsr04 can work on microibit v1, but it cannot work on microbit v2. Python editor version for my test is 2.1.0-beta.7. I think time_pulse_us or sleep_us has some bugs.

image

microbit-carlos commented 3 years ago

What's the result you get in a v1 vs v2?

crouse12 commented 3 years ago

microbit v1 show distance on led screen. However, microbit v2 does not show anything. I found that makecode has the same issue.

https://github.com/microsoft/pxt-microbit/issues/3731

crouse12 commented 3 years ago

I find v0.2.23 of codal-microbit-v2 now released. It solves pulse problem. Does micropython update, too?

https://github.com/microsoft/pxt-microbit/issues/3731

dpgeorge commented 3 years ago

Yes MicroPython is now updated to codal v0.2.23.

jaustin commented 3 years ago

@crouse12 could you please test with the build from https://github.com/microbit-foundation/micropython-microbit-v2/actions/runs/419939816 (this is an automatically built version from a commit on master, so isn't exactly what the next release will look like, but does have the updated CODAL).

crouse12 commented 3 years ago

I test time_pulse_us on microbit v2 using the following two MicroPython Versions. However, they cannot work. I think there are something wrong on time_pulse_us.

image

image

jaustin commented 3 years ago

@dpgeorge does the patch from @xmeow look okay to you? Would be great to close this off with the next release if so?

microbit-carlos commented 3 years ago

Looks like PR https://github.com/microbit-foundation/micropython-microbit-v2/pull/58 is adding the CODAL method to the MicroPython pin API. I think ultimately what we need is to make sure machine.time_pulse_us works on V2.

MakeCode also had an issue in this area, and the feature was originally implemented in the MakeCode code too, so the functionality has been moved to CODAL. It'd be good to use CODAL in this case as well to unify the implementation.

dpgeorge commented 3 years ago

machine.time_pulse_us() has been fixed by b7a76e449cf1b437972c21efaf468fbe5891c6a2 . So this function should now work the same as on V1.

It'd be good to use CODAL in this case as well to unify the implementation.

If the built-in MicroPython one works, is it ok to just keep that the way it is?

jaustin commented 3 years ago

I think if it's not a trivial switch over, I'm happy to leave things as they are with the V2 implementation.

microbit-carlos commented 3 years ago

Great, closing this as fixed. 🎉