-
Notifications
You must be signed in to change notification settings - Fork 280
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
Stepper rework as variable #73
Conversation
the variables seen in the dropdown Conflicts: blockly/blockly4Arduino/index.html blockly/blockly4Arduino/index_en.html
Step 1: the block Todo: onchange for errors in block & generator
Oh, this is really interesting! I've been meaning to refactor the current stepper implementation for ages, it was a "temporary workaround" for a workshop that ended up in the codebase far longer than anticipated. It's a bit late to have a proper look now, but i'll check it out as soon as I can. Quick question based purely on the general description, would this result in the stepper variable name to appear in the variable block dropdown? (I'll have a look at all the pending stuff as well as soon as I get a chance, apologies for the delay) |
Yes, I choose to have it appears in the variable block dropdown. Removing it there would mean more changes in core. As it contains a value (the pins the stepper is connected to), it can be a variable. For stepper it is array of two pins, which we cannot do much with, but for other components (button, ledstrip, ...) it is the pinnumber. I don't see a big need to work with these variables, but it is something I can think some interesting applications for. |
goog.provide('Blockly.Blocks.ComponentFieldVariable'); | ||
|
||
goog.require('Blockly.Blocks'); | ||
//goog.require('Blockly.FieldVariable'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this might be required, even if it compiles due to the closure parsing ordering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw other block files which depend on not mentioned requires which are present due to closure parsing, so commented it out. Originally I tried to have this present in blocks/arduino/ which is not possible due to dependencies, then this require is needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you can see with the new FieldNumber, it's good practice to explicitly indicate the dependency, if it works here I would leave it in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gives error in build version:
FATAL ERROR
required "Blockly.FieldVariable" namespace never provided
blocks/procedures.js at line 30:
goog.require('Blockly.FieldVariable');
^
For some reason the compiler gives wrong js file where the error happens ...
Thanks for the submission, looks quite good, I've added a few minor comments. I've been thinking about this and I quite like the idea of having a mechanism to store global "lists of things" that can be displayed on dropdowns, however I'm not quite sure they should be variables. In a lot of cases we are basically creating class instances, and while they are stored in variables, these are not variables that could be used in any other blockly "variable context" (variable blocks can be connected essentially everywhere). So I think we should separate this completely from the variable class, while mirroring the functionality and allowing to create different isolated categories. |
Well, the fact that they are variables fits with my overarching idea of assigning pins to variables, then using the variables to do writes/reads. So I need these variables to be able to do this, they should not be something special, but behave like the variables that have values of pins, so I can use them. My overall plan for how I want to have children work with our projects is like this:
|
If we mantain this idea of "instances of things" separated from variables, you could still assign a "thing instance" to a variable and use it as you are describing, no? |
Yes. Extra block needed on the canvas to describe it though. Although it are different things, variables already are different things now too (number, decimal, string ...). |
Ok, tackled most issues, inheritance can indeed be simplified. Not a js coder though, so I hope I follow the correct paradigm ... |
To have an idea how this looks when having many constructs that use this, see example: So, if you add a variable there, then the dropdown of the variable has all those names. Is that a problem or cumbersome? If you work with variables you typically set it once, and then use copy/paste to have other copies, so the fact that the dropdown contains more is not really a problem. If we have a way to identify which variables are pins, then all those should be present in the dropdownbox of a digitalwrite or read. |
Thanks a lot for all your contributions @bmcage . I'll finally have some time to look into this properly, for now I've merged into master and started playing with it locally, I'll let you know how it goes as I might think about creating this "instances of things" system based on the same concept as blockly variables. |
As I mentioned in the earlier discussions, I finally had some time to add some kind of "instance" implementation that can be used to create custom dropdowns.This also maintains it separated from variables, as otherwise these could end up being used in all kinds of "invalid" block configurations. It's not finished, and needs some work to fix a couple of minor issues, but the bulk is in there: c757d75 You can see that blocks can now do a @bmcage I was wondering what was the reason to add a global array for the stepper pins in this PR. |
The stepper is special in attaching to two pins. For components that attach to one pin, this global is an integer and can be easily used in blocks. So it is a design choice to always create a global with the pins of a component. If it was an LCD, it would be a longer array. In reality, a component hopefully has a library with one init function, and one can pass the pins once, and forget about them, making this global unneeded for actual use as the functions are then library functions. |
Stepper rework as variable
This PR is to convert the stepper construction to using the variable blocks.
Reasoning: