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

Variable pins #70

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Variable pins #70

wants to merge 6 commits into from

Conversation

bmcage
Copy link
Contributor

@bmcage bmcage commented Apr 13, 2016

This is a first attempt on how pins can be assigned to variables, and then the variable appears in the dropdown for digitalRead block.

There are some issues here which are to discuss:

1/ we need to test what typed variables are present for the dropdown in the block, but now statictyping is only instantiated in the generator. I hack this here so as to have it accessible in the io.js dropdown code also. No idea how to do this clean...

2/ Again, for the dropdown, we need to query the blocks to know what variables are present. I don't know how to obtain the workspace, I hack it with a call to Blockly.getMainWorkspace() ...

Also some extra cleanup needed: when deleting a variable, the block must be updated.
Also, to test in ardublockly, you need to expose the block 'io_pin_dig' and assign it to a variable.

The idea however on how to do this is present, so as to decide what is good approach to this. So, ideally you can use this PR as a start to provide a nice patch to allow this. Doing this in other blocks that have pins as input can then be done with future patches (analogread, pulsein, ....)

@bmcage bmcage mentioned this pull request Apr 14, 2016
@carlosperate
Copy link
Owner

carlosperate commented Apr 14, 2016

I'll see if I can do the code review later tonight. In the meantime, what do you think about having a single block with a dropdown to select the type of pin to assign?
This way it could also be easily expandable for additional blocks, like with the grove blocks that extend the types of pins available: https://github.com/carlosperate/ardublockly/blob/master/blocks/grove/extensions.js#L44-L50

@bmcage
Copy link
Contributor Author

bmcage commented Apr 14, 2016

Yes, 1 block with dropdown instead of 3 blocks would be a big improvement. For Grove, Microduino, ... additional pins seem indeed to exist, but probably better to keep those in an expansion, and not in the core block.

@carlosperate
Copy link
Owner

carlosperate commented Apr 15, 2016

Hi,

Sorry I ended up caught up with a couple of other things last night.

A general question I've got, that might conclude in some changes on the way pins are identified: Even though I haven't thought about it all the way through, I think I've always envisioned the pin type being "signal type agnostic". What I mean is that I generally conceptualise the idea in my head of having a single Blockly.Type for Pin and then distinguished between them in the Arduino generator. This would be done as a "per block generator" level, by calling the Blockly.Arduino.reservePin function. This simple vision from my part derived from having these features kind of growing with time, rather than with proper planning, and only really using the pin types to generate block warnings.

Your current approach creates a Blockly.Type per pin signal type, and I was wondering if that offers any advantage, moving that distinction to the block level. I'm asking not because I'm not open to the idea, but mostly because with my current perspective I was thinking about having a Arduino.Pins namespace similar to what the boards currently have, as these two a likely to be more linked in the future.

Edit: Was writting this message on mobile and accidentally pressed on "close PR" half way through. I've reopened it and updated the message text.

@bmcage
Copy link
Contributor Author

bmcage commented Apr 15, 2016

The reason I had for different pintypes is that in blocks, only specific pins can be used. So in motor block the dropdown is only PWM pins, in digitalwrite only digital pins, ... . As a consequence, if pins are assigned to a variable, the variable must know what type of pin so as to appear only in the correct dropdown.

One could argument that all variable pins must be in the dropdown, for more versatile use by experts. As the public I see using ardublockly are non experts, erring on the save side by keeping it save is the approach I choose, and hence a Type per use of pins. Like this a novice does not make an error.

Variables assigned with pin number like this also can be forced not to be used in computations or as values, which for novices is also a good thing. Looking in boards.js, having also spiPin and i2cPin, or for Grove pins, is an option.

This was referenced May 6, 2016
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.

3 participants