search

adafruit / pxt-crickit

pxt package for crickit platform - beta
MIT License
8 stars 4 forks source link

Add pinPullup/down block #17

Closed nicholas-gh closed 3 years ago

nicholas-gh commented 3 years ago

I'd quite like to better match the micro:bit makecode native pin pullup/down block, but I've not managed to figure out how to do that yet. It's probably better to do that work before merging, otherwise it would be unfortunate for any users of the block if it's changed.

By 'matching', I mean the visual style of selecting up/down/none (I currently have high/low) and making use of the enum PinPullMode:

https://github.com/microsoft/pxt-common-packages/blob/bb10eb307d9a34d9fc9b5b48d50eebd4bba841e0/libs/core/pinsDigital.cpp#L21-L28

https://github.com/microsoft/pxt-microbit/blob/9ea346bb363ae1d78f53ce3ec5220f62f4b82cfd/libs/core/_locales/core-strings.json#L399

This addresses #11

ladyada commented 3 years ago

ok i shall wait till ya do another pass at it?

nicholas-gh commented 3 years ago

ok i shall wait till ya do another pass at it?

Tips/direction would be handy, but if there's no one available for that, I'll work at it again soon on my own.

ladyada commented 3 years ago

@pelikhan may have some suggestions as they authored the library (tbh i barely know typescript)

pelikhan commented 3 years ago

This looks pretty good. Could you post a screen shot of the blocs?

nicholas-gh commented 3 years ago

To test changes, I'm finding I need to

Please can you guide me on how to have a local pxt editor simply load the (uncommitted) extension from local disk? https://makecode.com/extensions/getting-started/vscode#step-5-testing says like this:

{
    "name": "banana test",
    "dependencies": {
        ...
        "banana": "file:../pxt-banana"
    },
    ...
}

but I didn't get it working yet - extension doesn't turn up, and no errors are shown in the browser UI (and not that I spot yet in the pxt serve console output either). If you say that is the way, I'll try harder to see what's wrong (I'll trace file IO to see what it attempts to do and what fails). Would you expect an absolute full filesystem path to work there too? What about putting a file: URL in to the 'add extension' page?

nicholas-gh commented 3 years ago

Here's the pxt-crickit blocks vs. the pxt-microbit ones. I note the existing blocks, which I didn't add, already differ in look-and-feel from the pxt-microbit, so maybe actually the new block should be consistent with pxt-crickit (which I have) rather than pxt-microbit.

image

My new block is missing a 'clear pullup/pulldown' functionality, that probably needs adding before we merge.

nicholas-gh commented 3 years ago

My new block is missing a 'clear pullup/pulldown' functionality, that probably needs adding before we merge.

https://learn.adafruit.com/adafruit-seesaw-atsamd09-breakout/gpio PULLENCLR is defined in https://github.com/adafruit/pxt-seesaw/blob/6f5e163620e90225a858f2e0c0bee008ed79f635/seesaw.ts#L231 but not available without editing pxt-seesaw https://github.com/adafruit/pxt-seesaw/blob/6f5e163620e90225a858f2e0c0bee008ed79f635/seesaw.ts#L403-L418

So in the interest of having consistent use of high/low UI within crickit blocks, the 'clear' should be a separate block, therefore it can be done at a later date, I think I'd suggest merge of this PR without that functionality.

nicholas-gh commented 3 years ago

This looks pretty good. Could you post a screen shot of the blocs?

@pelikhan Thanks for taking a quick look. Screenshot added - https://github.com/adafruit/pxt-crickit/pull/17#issuecomment-760743857

nicholas-gh commented 3 years ago

@pelikhan Please may I ask for a merge? Is there anything I should do further towards that?

Thank you!

pelikhan commented 3 years ago

Should be ready to roll now