microsoft / pxt-microbit

A Blocks / JavaScript code editor for the micro:bit built on Microsoft MakeCode
https://makecode.microbit.org
Other
710 stars 586 forks source link

Digital pin vs analog pin blocks #5733

Closed microbit-carlos closed 1 week ago

microbit-carlos commented 1 month ago

Summary

The "analog pin" dropdown lets you select invalid pins for the [analog read pin [P0]] block, without showing any warning/errors, and it simulates it successfully, even though it will not work on the device.

Current situation

In MakeCode v6 we have pin blocks with two kinds of pin dropdown widgets:

The problem

This is all good for the digital read, digital write and analog write blocks. However, for the analog read block, the analog pin dropdown let's the user select an invalid pin, it doesn't show any warning on the block, and it simulates the ADC in the simulator:

image

This programme will not work on the device, as P9 is not an ADC pin.

New blocks

Since the introduction of pin parameters in PR #5700, we now also have the option to set a variable to a "digital pin" or "analog pin".

image

So this could add an additional layer of confusion to the user, as you can use the "wrong" pin block type and it will work in some cases, but might not in others.

This this example we are:

image

Suggestions

These are not final and are open for discussion.

We could:

sae220 commented 1 month ago

That's a nice suggestion, but we need a way to restrict block placed on top or to display erros.

@abchatra @riknoll Do you have any example or idea to solve this?

riknoll commented 1 month ago

@sae220 the only way to do this today would be to change the argument/return types to be different

abchatra commented 1 month ago

I am good with @carlosperate suggestion. @sae220 do you want to try and fix this?

microbit-carlos commented 1 month ago

Thanks @abchatra! To clarify, the suggestion from the description was just an initial idea, I do need to check with the rest of the team internally first, as it is a change to the public facing blocks. Due to the time constrains we were considering this as something that maybe we had to leave out for this release?

@sae220 for that reason if you could separate the work for https://github.com/microsoft/pxt-microbit/pull/5700#issuecomment-2233675975 from this one (into different PRs), that would be great, so that they don't block each other from being merged.