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!!!

from icestudio.

Thx - What other things should go in the Advanced features menu?

• Remote hostname - used by anybody? Code says: //-- TODO: Think about removing this function in future versions
• Python environment - is it needed in emergencies, trying to get Icestudio/Apio to work?
• External plugins - would they be, mainly, for using Icestudio/Verilog/simulation in an advanced way? Or will plugins be for look & experience & cool features of Icestudio, for SigRok, for test benches, for debugging - then I'd say they're of potential interest to anyone

So if my assessment is OK, I would put in the Advanced menu:

• Board rules (turning them off is quite unusual)
• Allow tri-state connections
• Remote hostname

from icestudio.

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!

from icestudio.

from icestudio.

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.

from 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.

from icestudio.

I've been doing test cases - there are a lot.
2 examples so we can talk specifically:

1. Existing project files accept "inout" appearing, when setting is disabled as above. One can do "Add block..." and the block contains "inout"s. Then Save and Save As will preserve the "inout"s in the model. I found that a bit surprising - but OK, fine.
2. A crash can come along "later" when one has a project like case 1 open. E.g., the added block can have all "inout" ports on the right side. In that case the "out" array is found to be undefined here in editBasicCode, when opening the block:
   let outPortNames = blocks.portsInfo2Str(block.data.ports.out);

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:

• recognize this mismatch as early as possible
• go with this plan:

they should be able to do so, or they should be prompted to enable it

• So I can prompt
  "You are adding code that uses "tri-state". You can only do this if you accept the design consequences/complexity of this feature by updating your user preferences: "Advanced features -> Allow tri-state connections". Click ("Update Preferences") or ("Just while this file is open"), or ("Cancel")"
• How about that?
  I just need to recognize as early as possible, by using a couple of locations in the code to handle all scenarios.
  And the flag for "Just runtime - do not save to profile.json", hopefully that doesn't have any edge cases.

from icestudio.

Hi @TimRudy ! give me the day to think in it please!

from icestudio.

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.

from icestudio.

This is great. I don't want to step on toes, we can collaborate. On the weekend I tried injecting the checks these places:

    function pruneBlock(block) {                <-- now call it pruneAndScanBlock(block) {
      // Remove all unnecessary information for a dependency:
      // - version, board, FPGA I/O pins (->size if >1), virtual flag
      delete block.version;
      delete block.design.board;
      var i, pins;                                           <-- and var isInoutPorts = false;
      for (i in block.design.graph.blocks) {
        if (block.design.graph.blocks[i].type === blocks.BASIC_INPUT ||
          block.design.graph.blocks[i].type === blocks.BASIC_OUTPUT ||
              ...
          delete ...

          if (block.design.graph.blocks[i].data.inout) {         <-- a check (the only check?)
            isInoutPorts = true;
          }
        }
      }
      return block;                            <-- now return { block, isInoutPorts };
    }

There are 2 places where pruneAndScanBlock(block) is called. In the following location, this.addBlock, I think block is basically added to the model; therefore it might be a bit too late. I needed to see how to reverse the load - or maybe there is a better location to use:

block = _safeLoad(block);
let { block: newBlock, isInoutPorts } = pruneAndScanBlock(block);
if (isInoutPorts) doPrompt()
return early, or not

Thanks for taking a look at it

from icestudio.

Test Cases:

1. Regression tests:

• Open, New, add & edit components, navigate in and out & unlock, edit, lock, Save, Save as
• Bring in blocks using Add as block, Paste, drag from Collection Manager
• Copy, Paste, Clone, Duplicate
• Content fit is done at right time after opening designs, imports, navigate in and out, window resize
• Loading animation is shown and dismissed at right times when action is taken & when transition happens

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")
* Prepare an .ice file with a Code block in it, that in turn has "inout" port (can be Left or Right)
* Place an .ice file with "inout" into a folder under user.../.icestudio/collections/<IceCollection>/blocks/ so it will appear in Collection Manager

* 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

Next 2 tests show:
Normal files & normal user's workflow will never trigger the "Tri-state"/"Advanced" warning

2. Start new version of Icestudio.

-> Profile at user.../.icestudio/profile.json picks up "allowInoutPorts: false"

3. Open normal file. Also New. Also Add as block (normal block). Do edits.

A) Add & edit port.
B) Add & edit Code block.
-> No fields are displayed for "inout"
C) Save, and Save as.
-> Get normal file (no "inout" data is written out)
D) Similar tests, dragging a block from Collection Manager. Save, Save as.
E) Similar tests, navigating into a block. Save, Save as.
F) Similar tests using Paste, Duplicate, Clone. Save, Save as.

Next 2 tests show:
New prompt displayed for "inout"; normal user when they do not upgrade their profile

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.
-> New prompt is displayed
-> Nothing happens to the design
-> Loading animation is dismissed if applicable
-> Other functionality works as normal afterward
-> Profile at .../profile.json still has "allowInoutPorts: false"

5. Open normal file. Do an edit. New to open a second window for Copy & Paste test.

Normal file opened is window #1.
In window #2, Open a block that has "inout". Respond "OK" and "This time" to the prompt.
(*i.e. no change to profile on disk)
Ctrl-C to Copy the component that has "inout".
A) In window #1, Ctrl-V to Paste the "inout" block.
-> "Tri-state connections" prompt is triggered in window #1 because both windows are "aware" independently
B) "Cancel". Also repeat Paste & "Cancel". Save as. Quit window #1. Then Quit window #2. Also repeat, #2 then #1.
-> On "Cancel", nothing happens to the design
-> On Save as, get normal file (search "inout", no result)
-> After any Quit, profile at .../profile.json still has "allowInoutPorts: false"
(*i.e. if never accept the "Tri-state connections"/"Advanced" Preferences change, never changes profile)
C) During same test, in window #2, Ctrl-V, Ctrl-Shift-V to Paste, Paste Clone.
-> There is no prompt, paste works fine
D) During same test, open a new window #3, from window #2, from window #1. Paste. Also open a block that has "inout".
-> There is a prompt; window #3 is independent
-> After any Quit, profile at .../profile.json still has "allowInoutPorts: false"

Next test shows:
New prompt; responding to prompt in 2 open Icestudio windows, or after close & re-open Icestudio;
  when user does upgrade their profile;
  when user views "inout" only temporarily, then re-opens Icestudio

6. Open normal file. New to open a second window, as above.

Normal file opened is window #1.
In window #2, open a block that has "inout". Respond "OK" and "Yes".
(*i.e. a change to profile on disk)
A) Quit window #2. Or Edit, Save then Quit window #2. Then Quit window #1.
B) Quit window #1. Then Quit window #2.
-> The windows still act independent (a window opened before the accept with "Yes" is not
"aware" of the new Advanced preference setting)
-> The order of closing windows doesn't matter. The accept with "Yes" is saved, and it persists

   (* The behaviour is not perfect. Each window (ideally) would subscribe and get a
       notification to update its "inout" setting to the saved profile.
       This would remove "unnecessary" prompts. It would make the Preferences menu
       "Advanced features -> Allow tri-state connections" consistent between the 2 windows)
   (* Another behaviour: When prompt "Yes" is saved (profile is saved) in window #1,
       then there is a prompt in window #2 but the response is "This time", it changes the
       profile back, undoing what was "persisted" from window #1)
   - But the above are edge cases

User with "Allow tri-state connections" == true

Next test shows:
New Advanced mode for tri-state: Activating, persistence, the new fields during editing

7. Open "inout" file. Also Add as block. Also drag from collection in Collection Manager.

This time respond with "OK, not "Cancel".
A) Respond "Yes" in 2nd prompt (upgrade to Advanced).
-> Any further actions with "inout" blocks are normal, there is no prompt
-> Profile at .../profile.json changes to "allowInoutPorts: true"
B) Same test as (A). Quit Icestudio. Open normal file this time. Also New.
Add & edit Basic Input, Output. Add & edit Code block.
-> Checkbox for "inout" is still displayed in port dialog
-> Fields for left and right "inout" ports are still displayed in Code block dialog
-> Profile at .../profile.json remains "allowInoutPorts: true"
C) Respond "This time" in 2nd prompt.
-> Any further actions with "inout" blocks are normal, there is no prompt
-> Profile at .../profile.json still has "allowInoutPorts: false"
D) Same test as (C). Quit Icestudio. After starting Icestudio, drag from collection again.
-> Prompt is displayed again
E) Same test as (C). Quit Icestudio. Open normal file this time. Also New.
Add & edit Basic Input, Output. Add & edit Code block.
-> No checkbox is displayed in port dialog for "inout"
-> No fields are displayed in Code block dialog for left or right "inout" ports
-> No prompt is displayed

from icestudio.

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?

You are loading a design that uses "tri-state". Tri-state (aka high-Z, bidirectional, or inout) ports are not recommended in standard designs.
You will be asked to update your Preferences (Advanced user setting) or you can just open this design on a preview basis.
Continue?

2nd version:

You are importing a block that uses "tri-state". ...<remainder the same>
Continue?

Then 2nd dialog:

Click "Yes" to allow tri-state and update Preferences:
   **Advanced features -> Allow tri-state connections**
Click "This time" to view tri-state for this design only.

from icestudio.

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!

from icestudio.

You're welcome! Message me if you want me to help on one of the tasks on your plate

from icestudio.

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!

from icestudio.