-
Notifications
You must be signed in to change notification settings - Fork 251
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
Hide or enable the inout feature so it only appears for "advanced" users? #729
Comments
Hi Tim, thanks again for your feeback !! I like a lot your proposal, I prefer the second choice but with "Advanced features" -> "Allow tri-state connections" The choice of options for power users like a lot but i think "power users" could be confused for some people and could be newbie user need tri-state and naturally not identify it with "advanced users". Do you like it? Go ahead with the PR!!! |
Thx - What other things should go in the Advanced features menu?
So if my assessment is OK, I would put in the Advanced menu:
|
Hi @TimRudy ! for the moment if you want, put Python environment too in advanced tab , only is used if you have your own python installation in some non standard directory or outside the PATH environment. External plugins will be a great feature for the next big update (very soon), i'm polishing plugin architecture to make easy that all of you could create new functionalities very easy, put in the advanced menu too. Thanks again! |
Hi Carlos,
Should I bump the project version
common.js: VERSION = “1.3” ?
Also another thing I thought of: There’s not a strong reason for this: But because of “inout” being an optional concept, I can take all the “inout”: false occurrences out of proj file at “prune” time. Only save “inout”: true occurrences.
… On Apr 3, 2024, at 2:11 PM, Carlos Venegas Arrabé ***@***.***> wrote:
Hi @TimRudy ! for the moment if you want, put Python environment too in advanced tab , only is used if you have your own python installation in some non standard directory or outside the PATH environment.
External plugins will be a great feature for the next big update (very soon), i'm polishing plugin architecture to make easy that all of you could create new functionalities very easy, put in the advanced menu too.
Thanks again!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
Hi @TimRudy! At the moment we do not change the version of the project, I have planned it for the near future for other reasons, but at the moment it could be problematic to change it. For this I want to adjust more things, version control within icestudio. |
Thinking about it, I just realized a problem. If a user has this setting disabled and opens a design that requires inout ports, they should be able to do so, or they should be prompted to enable it. The easiest thing would be to have three states for the inputs, with one more class it is simple. We would have the normal state, the hidden state (they are not displayed) and the "disabled" state. What I would do is remove them disabled and with a degree of opacity, for example, 0.6 so that they can be seen but cannot even be changed and understood. visually it is somewhat disabled. What do you think? It is important that these new options are only for visualization and do not affect the project itself. |
I've been doing test cases - there are a lot.
Scenario 2 is just code that can be fixed because it's not robust enough. However, considering case 1, I can understand why you are talking about the "disabled" visual state. It's because you know from experience that "inout" is not considered "Invalid project format". It should be "tolerated", it is kind of expected. Correct? So: I don't think "easiest" is the word I'd use for having a third "disabled" visual state... :-) Is it good for the user? What is easiest for the user? If a "no-tri-state" user opens a project file or adds a block that has "inout" ports, how about this:
|
Hi @TimRudy ! give me the day to think in it please! |
I've been looking into it and let me know what you think about the following. Regarding "tolerance", you're right, the model is not "strict," which leaves open situations like these that are relatively flexible, though not 100% robust. I wanted to go over it because I'm making changes in this regard and didn't want to "step on toes." What do you think if I provide you with some functions that tell you if there are inouts in the design, so you could directly check during loading, and if it returns true, display the message and activate them? About showing them or not, I agree with you, upon further reflection, it's better that the user doesn't see anything about inouts unless they activate it. I think the idea of having a state to enable inouts only during execution is very good. If you agree, I'll prepare this first version of the "linter" with that check, and you tell me how you see it. |
This is great. I don't want to step on toes, we can collaborate. On the weekend I tried injecting the checks these places:
There are 2 places where
Thanks for taking a look at it |
Test Cases:1. Regression tests:
For next tests:* Prepare an .ice file with "inout" port in it, can be Basic Input, Basic Output (this does not include "IceIO" collection devices, which declare the ports without using "inout") * Note existing Icestudio behaviour is to accept data elements "inout", "inoutLeft", "inoutRight" in the existing file format. Even though Verify and Build may fail, those data elements are imported, copied around, saved transparently. * This PR leaves the file format at version "1.2" * In the following, "normal" file or block means => no "inout" data User with "Allow tri-state connections" == false
2. Start new version of Icestudio.-> Profile at 3. Open normal file. Also New. Also Add as block (normal block). Do edits.A) Add & edit port.
4. Open "inout" file. Also Add as block ("inout" block). Also drag "inout" block from collection in Collection Manager.Always respond with "Cancel". Repeat after the cancel; cancel again. 5. Open normal file. Do an edit. New to open a second window for Copy & Paste test.Normal file opened is window #1.
6. Open normal file. New to open a second window, as above.Normal file opened is window #1.
User with "Allow tri-state connections" == true
7. Open "inout" file. Also Add as block. Also drag from collection in Collection Manager.This time respond with "OK, not "Cancel". |
Here is proposed text (when a user first sees a tri-state component/.ice file): Would you translate it to Spanish and get a feeling for any changes/improvements you want?
2nd version:
Then 2nd dialog:
|
#729: Hide or enable tri-state so it only appears for "advanced" users
Hi @TimRudy ! i'm trying it this days and all works very well for the moment! thanks a lot of newbie folks gratefully you a lot! |
You're welcome! Message me if you want me to help on one of the tasks on your plate |
Thank for all @TimRudy , i'm publish my roadmap very soon, i hope that you join to some tasks if you want! In other hand i'm thinking in use your 74s collection to do an interesting example like a 1-bit cpu or something like this in an hybrid environment between icestudio/web/fpga could be very interesting if we start a thread if you want and propose a collaborative work around it. Think in it! |
For the moment i think this task is finished . if is needed we reopen it. |
To appeal to different users (learners and others), I was thinking Icestudio could have an "Advanced" type of sub-menu in Preferences.
The new feature inout ports would be good for this, because who is going to want inout ports? For most people, they won't have to see the "InOut pin" (the 3rd check box) when creating/editing an Input or Output.
Do you want this? I can open a PR putting the "Include or Don't include" menu choice right in the Preferences menu. Or, as in the second screen shot, I could create an "Advanced" sub-menu with 2 or 3 items in it.
Please let me know which text you would choose, e.g.:
P.S. I've thought about issues like this: "Normal" user opens a design that has inout ports in it. Icestudio would internally toggle to show inout ports, without further action. Most easily, this would mean: The user's profile would also update, storing the "Include inout ports" flag without further action. (They probably would not mind, and they could change it back afterward.)
This second menu idea could be a direction to go, what do you think? Name "Power user", or "Advanced...":
The text was updated successfully, but these errors were encountered: