Highlight Nets

[nobody]

It would be absolutely wonderful if there would be some highlight of nets (wires and/or busses). Viewing nets will gladly keep the program in great condition. Keep in mind this is a feature, a great feature that most programs of its kind currently have and certainly adds another professional look. Attached is an example of the project I am working on with KiCAD.

When a user right clicks on a wire or bus shape, add one display member to the drop down box that says "View Net".
After the user clicks "View Net", all surrounding cables are highlighted with the shown picture color.

Regards,
Juan Miguez

[nobody (nobody-users)]

The file Highlight Nets.jpg was added: Example of Viewing Nets, highlighting wires in EESchema

[mrluzeiro (mrluzeiro-users)]

Add highlight to PCBeditor too...

[Nicklas Björk (nicklas-3)]

In Pcbnew, highlighting of a net could be triggered by mouseover, with the option to turn it off.

[Darwish (darwish-d-d)]

To improve usability, please, add "net highlight" feature into EESchema editor.
Also, please, add proper hotkey. I'll prefere that it would be 'h'. According to my hardware development expirience, "net highlight" is more demanded than "Add Hierarchical Label"

Currently there is no this extremely useful functionality.

Application: kicad
Version: (2015-03-03 BZR 5469)-product Release build
wxWidgets: Version 3.0.0 (debug,wchar_t,compiler with C++ ABI 1002,GCC 4.6.3,wx containers,compatible with 2.8)
Platform: Linux 3.2.0-77-generic x86_64, 64 bit, Little endian, wxGTK
Boost version: 1.54.0
         USE_WX_GRAPHICS_CONTEXT=OFF
         USE_WX_OVERLAY=OFF
         KICAD_SCRIPTING=OFF
         KICAD_SCRIPTING_MODULES=OFF
         KICAD_SCRIPTING_WXPYTHON=OFF
         USE_FP_LIB_TABLE=HARD_CODED_ON
         BUILD_GITHUB_PLUGIN=ON
         KICAD_USE_WEBKIT=OFF

[Rainer (rwahler)]

I use KiCad version BZR6310 and this feature still seems missing. This bug is open for more than 6 years and nothing happened since then which is really sad.

I think highlighting wires in EEschema is a really needed feature. Especially when i think of busses where wires with different names get automatically connected to each other (see example in tutorial at http://docs.kicad-pcb.org/en/getting_started_in_kicad.html#bus-connections-in-kicad where a[] and b[] connects) and i have no possibility to really "see" which wires are connected makes schematic entries for complex circuits very hard to verify. I see the late additions to KiCad from CERN which are great features, but i would really recommend to implement the basic features which many people needs instead of advanced features which very less people need.

[Felix Vollmer (felixvollmer)]

AFAIK it is planned to switch the rendering of EEschema to the new GAL (introduced by CERN). This won't happen before the next stable. Since the rendering engine will change, there is little point to add this feature before. But I'm sure that contributions would be welcome.

I personally also would love to have this feature.

[robotarmy (ry-white)]

Yes!

Also, if you manage to get it working across a hierarchy, you'll be doing even better than "that un-named tool I use at work"

R

[jean-pierre charras (jp-charras)]

Yes, we want it working across a hierarchy.
(if limited to the current sheet it is easy to code but it is useless, and can even gives false info).

This is not really a matter of rendering, but more a matter of netlist generation (which needs to be very fast for this feature).

Refactoring Eeschema is currently a (not so easy) work in progress, and one of this refactoring reason is to facilitate implementation of this feature, among others features.

@Rainer
It is more easy to write "I want that since a long time" that making "that" working.

[Nox_firegalaxy (nox-7bitfaster)]

While the refactoring is going on I would like to throw in a little patch which adds net highlighting for eeschema. Right now an additional button is added in eeschema but maybe adopting the crtl+leftclick semantic from pcbnew is a better alternative?
Maybe this patch can be applied if a minor in between update is made?

[Rainer (rwahler)]

@Nox_firegalaxy:
I applied your patch and it works like a charm for me. Thanks for the patch for a feature request which is open for more than 7 years. :)

I would also vote for including this patch upstream until somebody comes up with a better approach.

Points i would improve a little bit when applying upstream:
1. A little better handling for unselecting a signal
2. Possibility to select the color for the highlighted net.
3. Maybe use the same symbol like in pcbnew for highlighting.

I don't care very much about these points but i think to use it more comfortable for other people it could matter.

[Nox_firegalaxy (nox-7bitfaster)]

@Rainer thanks for verifing the functionality and the feedback. I missed to remove the dialog if no node is selected.
Regarding your comments:
1. how would you handle the deselection?
2. I was thinking about that but wanted to keep the necessary changes to a minimum to not interfere with the refactoring (and I had no idea where to place this "option")
3. I could not figure out the name of the symbole (the search took me to long so I skipped it) - if someone has the name I can change it

[Rainer (rwahler)]

The useless dialog is the only thing which is a bit disturbing. To click just in a free area to unselect the signal is ok for me.

For point 2 and 3 maybe some developer can comment on it when they are willing to integrate. Or just let it be like it is as a temporary solution to this feature request. Better to highlight somehow than not at all. ;)

[Nox_firegalaxy (nox-7bitfaster)]

Found the button icon and removed the annoying dialog.

[Rainer (rwahler)]

I tested the new patch and it works very nice. Thanks for your work. :)

[Nox_firegalaxy (nox-7bitfaster)]

new version with better bus support (far from perfect).

[Chris Pavlina (pavlina-chris)]

Wayne, what do you think? I'd really like to merge this.

I like this a lot, and it works _very_ nicely. I have one more request: it seems to me that the bus quirk could be resolved pretty simply by disallowing selecting a bus directly. Could you amend it to do that?

[jean-pierre charras (jp-charras)]

It is nice.

However there are issue:
* If a highlighted net has labels, there is a serious issue (perhaps because they have some flags set).
* It does not work on complex hierarchies.
Complex hierarchies are always very tricky, because each item is stored only once in schematic, and has different properties depending on the sheet path (reference for instance)
It means one cannot store the highlight flag (on/off) just in a boolean.

Minor issues:
* Disallowing selecting a bus directly: OK
* A net should be selectable by clicking on any item (especially labels) and labels should be highlighted (it is also a fix for bus)
* Do not hard-code the highlight color.

[Nox_firegalaxy (nox-7bitfaster)]

Could you elaborate on the "label" and the "not work on complex hierarchies" issue?

[jean-pierre charras (jp-charras)]

First, thanks for your interest at Kicad.

Regarding labels:
If a net with labels is highlighted, try to right click on a label .
It shows option for label during edition (Cancel option).
I am guessing this is because the highlight option is stored in m_Flags, used mainly in edition and local calculations in Eeschema.

Regarding complex hierarchies:
Complex hierarchies are a *tricky* case.
If a wire (or label) is flagged as highlighted, it is highlighted in any instance of the sheet.
This is obviously false, because if the net is local, it must be highlighted only for one instance, not for all instances.
Moreover, if a net is connected to a root sheet, it must be highlighted for the corresponding instance, not for all instances.

Therefore the "highlight" state cannot be stored inside a schematic instance without identifying the sheet instance(s) (they can be more than one), because the same item can be highlighted or not, depending on the sheet instance) (or, if you prefer, the sheet path), or it must be updated when you switch from one sheet to an other sheet.

[Nox_firegalaxy (nox-7bitfaster)]

I will look into the label stuff.

Regarding the wrong highlighting: can you provide an example file for that? I am curious as the patch uses the generated netlist and just select all nets with the same netcode. So I can hardly figure a case where this approach would "break" as long as the generated netlist is correct.

[jean-pierre charras (jp-charras)]

You can have a look at complex_hierarchy demo.
This is a very basic case. You can play with it.

You are wrong when writing:
"uses the generated netlist and just select all nets with the same netcode"
The generated netlist creates a *flat* hierarchy which does not exist in a complex hierarchy.

You do not use "all items with the same netcode"
You are setting a flag in a shared schematic sheet item, which is not the same thing.

In complex hierarchies, this item belongs a netcode depending on the sheet path.
Therefore the *same item* belongs different netcodes depending on this sheet path and it is not a fixed netcode.
This is also true for references: a given component has different references, depending on its sheet path.

[Wayne Stambaugh (stambaughw)]

On 10/5/2016 3:23 AM, jean-pierre charras wrote:
> It is nice.
>
> However there are issue:
> * If a highlighted net has labels, there is a serious issue (perhaps because they have some flags set).
> * It does not work on complex hierarchies.
> Complex hierarchies are always very tricky, because each item is stored only once in schematic, and has different properties depending on the sheet path (reference for instance)
> It means one cannot store the highlight flag (on/off) just in a boolean.
>
> Minor issues:
> * Disallowing selecting a bus directly: OK
> * A net should be selectable by clicking on any item (especially labels) and labels should be highlighted (it is also a fix for bus)
> * Do not hard-code the highlight color.
>

I will add:

* Coding policy violations.
* Assign it a hot key. Constantly moving the cursor out of the drawing
area to select a tool from the toolbar is cumbersome.

[Chris Pavlina (pavlina-chris)]

> * Assign it a hot key. Constantly moving the cursor out of the drawing
area to select a tool from the toolbar is cumbersome.

The matching tool in pcbnew does not have one. I'd assume the two should have similar behavior; should pcbnew be given a hotkey for this too?

Wayne, re: hotkeys, I want to make a quick suggestion. We're running out... I think we ought to make the hotkey editor allow blank/unset hotkeys, so that we can make more advanced features able to have hotkeys if the user wants them without having to cram them into the layout. This way users can edit their hotkey configurations and exchange ones they don't use for ones they do.

Feel free to split this off into another bug or ML thread if this requires further discussion.

[Nox_firegalaxy (nox-7bitfaster)]

regarding the right click on label stuff:
did you test it only with the new tool or with another tool as well? Because you are right that there is a cancel option but as far as I can see this is case for the other tools as well.

To the wrong highlighting:
Now I understood thats your point after taking a look at the demo file - a "EDA_ITEM" only exists once per sheet. So if multiple sheet instances of the same sheet exists they share the same item. This is indeed a serious issue and not so easy to tackle obstracle. To find there a proper handling of this case will be mostlikely very bloaty. But I am looking into it right now. Thanks for bringing that up.

Hotkey:
I wanted to make the patch as slim as possible and maybe later provide a hotkey as an independent patch.

[Wayne Stambaugh (stambaughw)]

On 10/5/2016 9:22 AM, Chris Pavlina wrote:
>> * Assign it a hot key. Constantly moving the cursor out of the drawing
> area to select a tool from the toolbar is cumbersome.
>
> The matching tool in pcbnew does not have one. I'd assume the two should
> have similar behavior; should pcbnew be given a hotkey for this too?

Ideally, yes.

>
> Wayne, re: hotkeys, I want to make a quick suggestion. We're running
> out... I think we ought to make the hotkey editor allow blank/unset
> hotkeys, so that we can make more advanced features able to have hotkeys
> if the user wants them without having to cram them into the layout. This
> way users can edit their hotkey configurations and exchange ones they
> don't use for ones they do.

I think this is a good idea. We are definitely running short of hot
keys unless we start using the function buttons along with modifier keys
but that's not ideal.

>
> Feel free to split this off into another bug or ML thread if this
> requires further discussion.
>

[Nox_firegalaxy (nox-7bitfaster)]

@charras nvm I now realized what you meant about the label stuff. Will look into that too but first things first.

[Nox_firegalaxy (nox-7bitfaster)]

its getting more and more bulky that thing. But maybe the major issues are now solved. I did not add hotkey and flexible colors as the patch is right now already quite invasive. Labels are currently not highlit as they seems to behave differently i.e. the deselections does not work correctly for them.

[Chris Pavlina (pavlina-chris)]

Actually, on hotkeys:

Having given this some thought, I disagree that it should have a *hotkey* (and that one should be added to pcbnew as well). GAL now has a shortcut for this, but it isn't a hotkey, it's ctrl-click. Since this was selected as the way to highlight nets in GAL, perhaps we should carry that through into pcbnew legacy and eeschema. Wayne?

Nox: I haven't had a chance to test the updated patch yet, but on the patch getting bulky - personally, I don't feel it's excessively large. Actually I feel you've done a good job of keeping things minimal - I only see two "large" blocks of code added, and they're only a couple dozen lines each. Plus the necessary glue to tie those into the UI. I personally would not worry about the patch size at this point.

[Wayne Stambaugh (stambaughw)]

On 10/6/2016 3:28 AM, Chris Pavlina wrote:
> Actually, on hotkeys:
>
> Having given this some thought, I disagree that it should have a
> *hotkey* (and that one should be added to pcbnew as well). GAL now has a
> shortcut for this, but it isn't a hotkey, it's ctrl-click. Since this
> was selected as the way to highlight nets in GAL, perhaps we should
> carry that through into pcbnew legacy and eeschema. Wayne?

I'm fine with this. That means that no additional tool required to make
this work. Just use the standard select tool plus the ctrl key.

>
> Nox: I haven't had a chance to test the updated patch yet, but on the
> patch getting bulky - personally, I don't feel it's excessively large.
> Actually I feel you've done a good job of keeping things minimal - I
> only see two "large" blocks of code added, and they're only a couple
> dozen lines each. Plus the necessary glue to tie those into the UI. I
> personally would not worry about the patch size at this point.
>

[Chris Pavlina (pavlina-chris)]

On Thu, Oct 06, 2016 at 01:02:02PM -0000, Wayne Stambaugh wrote:
> On 10/6/2016 3:28 AM, Chris Pavlina wrote:
> > Actually, on hotkeys:
> >
> > Having given this some thought, I disagree that it should have a
> > *hotkey* (and that one should be added to pcbnew as well). GAL now has a
> > shortcut for this, but it isn't a hotkey, it's ctrl-click. Since this
> > was selected as the way to highlight nets in GAL, perhaps we should
> > carry that through into pcbnew legacy and eeschema. Wayne?
>
> I'm fine with this. That means that no additional tool required to make
> this work. Just use the standard select tool plus the ctrl key.

Perhaps if I get the time I will make pcbnew legacy work this way and
remove the tool. I'm wondering if we should split this commit: accept
Nox's patch working the way it does, which matches pcbnew legacy (as
eeschema usually does), then edit both pcbnew legacy and eeschema to
work like pcbnew GAL in a separate commit.

Also slightly offtopic, I wonder if something should be done to make
things like this more obvious. I really like the convenience of
ctrl+click in GAL, but had absolutely no idea it was there until one of
the GAL devs told me. A lot of software that makes heavy use of
modifiers displays a summary of active cursor modkeys and their meanings
in the status bar...

>
> >
> > Nox: I haven't had a chance to test the updated patch yet, but on the
> > patch getting bulky - personally, I don't feel it's excessively large.
> > Actually I feel you've done a good job of keeping things minimal - I
> > only see two "large" blocks of code added, and they're only a couple
> > dozen lines each. Plus the necessary glue to tie those into the UI. I
> > personally would not worry about the patch size at this point.
> >
>
> --
> You received this bug notification because you are a member of KiCad Bug
> Squad, which is subscribed to KiCad.
> https://bugs.launchpad.net/bugs/594073
>
> Title:
> Highlight Nets
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/kicad/+bug/594073/+subscriptions

[Wayne Stambaugh (stambaughw)]

On 10/6/2016 9:24 AM, Chris Pavlina wrote:
> On Thu, Oct 06, 2016 at 01:02:02PM -0000, Wayne Stambaugh wrote:
>> On 10/6/2016 3:28 AM, Chris Pavlina wrote:
>>> Actually, on hotkeys:
>>>
>>> Having given this some thought, I disagree that it should have a
>>> *hotkey* (and that one should be added to pcbnew as well). GAL now has a
>>> shortcut for this, but it isn't a hotkey, it's ctrl-click. Since this
>>> was selected as the way to highlight nets in GAL, perhaps we should
>>> carry that through into pcbnew legacy and eeschema. Wayne?
>>
>> I'm fine with this. That means that no additional tool required to make
>> this work. Just use the standard select tool plus the ctrl key.
>
> Perhaps if I get the time I will make pcbnew legacy work this way and
> remove the tool. I'm wondering if we should split this commit: accept
> Nox's patch working the way it does, which matches pcbnew legacy (as
> eeschema usually does), then edit both pcbnew legacy and eeschema to
> work like pcbnew GAL in a separate commit.

I'm fine with this as long as it is going to get fixed. I want to avoid
another drive by patch that never get finished because it satisfies the
needs of the person who submitted the patch but has no intention of
seeing it through the end.

>
> Also slightly offtopic, I wonder if something should be done to make
> things like this more obvious. I really like the convenience of
> ctrl+click in GAL, but had absolutely no idea it was there until one of
> the GAL devs told me. A lot of software that makes heavy use of
> modifiers displays a summary of active cursor modkeys and their meanings
> in the status bar...

I'm guessing you could add a help string to the status bar or maybe a
more descriptive tooltip.

>
>>
>>>
>>> Nox: I haven't had a chance to test the updated patch yet, but on the
>>> patch getting bulky - personally, I don't feel it's excessively large.
>>> Actually I feel you've done a good job of keeping things minimal - I
>>> only see two "large" blocks of code added, and they're only a couple
>>> dozen lines each. Plus the necessary glue to tie those into the UI. I
>>> personally would not worry about the patch size at this point.
>>>
>>
>> --
>> You received this bug notification because you are a member of KiCad Bug
>> Squad, which is subscribed to KiCad.
>> https://bugs.launchpad.net/bugs/594073
>>
>> Title:
>> Highlight Nets
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/kicad/+bug/594073/+subscriptions
>

[jean-pierre charras (jp-charras)]

@nox,
Thanks for your patch 05.
this is better for complex hierarchies, but not yet fully working in more complex cases.
The labels issue is not fixed.

I am thinking you are using m_Flags to store the highlight status and this is not a good idea.
m_Flags is intended to temporary store flags mainly during edition and calculations, not for status bits.

You could try to use m_Status (GetState and SetState) to store the highlight status.
Note also a similar status flag exists for Pcbnew (BRIGHTENED) and accessors are already existing (IsBrightened(), SetBrightened ... ), so perhaps no need to create a new flag for Eeschema.

I uploaded a more complex hierarchy test (although this hierarchy is still basic) which shows highlight issues in the most inner sheets.

Thanks for your work.

[Nox_firegalaxy (nox-7bitfaster)]

@ charras you do this on purpose, right ;-P
this one really got me as it basically occurs for nested sheets (so one shortened C in the second level and nothing else is already enough to reproduce this behavior).
It turned out that I was once more hocused by kicads very intuitive data structure (m_SheetPath vs m_SheetPathInclude).

"I'm fine with this. That means that no additional tool required to make
this work. Just use the standard select tool plus the ctrl key."

Well pcbnew has a tool icon for highlighting as well does not it? Adding a shortcut is of course no big deal if it would be clear which way is the most canonical one (which is not obvious for me). Actually I did not know about the ctrl + click stuff in pcbnew for a long period. But imho no highlighting at all ist far worse than the need to select a tool first by mouse.

I noticed that there is a hotkey file somewhere. Would it be possible to add HK_LEFT_CLICK_CTRL, HK_LEFT_CLICK_SHIFT HK_LEFT_CLICK_ALT etc and use the hot key binding file (tbh I currently do not understand how pcbnew is handling the ctrl+lmf right now)?

P.S. quite an effort for patching up some subtool which is tagged as legacy and mostlikely refactored anyway...

[Nox_firegalaxy (nox-7bitfaster)]

P.P.S. i hope switching from mFlags to mStatus did not break some other things but I could not find any accessing code so far.

[Nox_firegalaxy (nox-7bitfaster)]

newest version supports know highlit labels and color selection/setting. If you can name a free hotkey I can add it as well.

[Rainer (rwahler)]

I tested version 07 and it looks very nice with highlighted labels and changeable color. :)

[Nox_firegalaxy (nox-7bitfaster)]

Hi jean,

as you seems to be most experienced, can you please review the latest patch?

Thanks in advance,

Nox

Am 06.10.2016 um 18:26 schrieb jean-pierre charras:
> @nox,
> Thanks for your patch 05.
> this is better for complex hierarchies, but not yet fully working in more complex cases.
> The labels issue is not fixed.
>
> I am thinking you are using m_Flags to store the highlight status and this is not a good idea.
> m_Flags is intended to temporary store flags mainly during edition and calculations, not for status bits.
>
> You could try to use m_Status (GetState and SetState) to store the highlight status.
> Note also a similar status flag exists for Pcbnew (BRIGHTENED) and accessors are already existing (IsBrightened(), SetBrightened ... ), so perhaps no need to create a new flag for Eeschema.
>
> I uploaded a more complex hierarchy test (although this hierarchy is
> still basic) which shows highlight issues in the most inner sheets.
>
> Thanks for your work.
>
>
> ** Attachment added: "test_complex_hierarchy.zip"
> https://bugs.launchpad.net/kicad/+bug/594073/+attachment/4756102/+files/test_complex_hierarchy.zip
>

[Nox_firegalaxy (nox-7bitfaster)]

To avoid stalling maybe someone else can review the latest patch? If I am not mistaken, all objections except for the hotkeying should have been taken care of. The short cut can only be dealt with if a decision is made which way it should be handled.

[Wayne Stambaugh (stambaughw)]

On 10/30/2016 8:04 AM, Nox_firegalaxy wrote:
> To avoid stalling maybe someone else can review the latest patch? If I
> am not mistaken, all objections except for the hotkeying should have
> been taken care of. The short cut can only be dealt with if a decision
> is made which way it should be handled.
>

I finally had a chance to test this it seems to work OK. I did not test
it on a complex hierarchy so maybe JP can take a look at that and
confirm that it works. One annoyance that I found is that on the first
net selection, the mouse warps back to the center of the drawing. This
needs to be fixed by setting the last position to the net selection
position but I'm fine if you do that as a separate patch.

There are a few coding policy issues that need to be resolved:

* Use "if( " not "if (" and "for( " not "for (".
* There are if statements on a single line. The body of the if
  statement must be on a separate line and indented.
* Your editor is leaving trailing white space.

Once the coding policy issues are cleaned up and I get confirmation that
it works on complex hierarchies, I will commit this.

[jean-pierre charras (jp-charras)]

It works on a complex hierarchy.

But I found an other serious (blocking) issue: if there is a not yet annotated component in a hierarchy (even a simple hierarchy) the annotation dialog is always opened when trying to enter or leave a sheet (regardless the highlight tool is activated or not).

[Wayne Stambaugh (stambaughw)]

On 10/31/2016 3:19 PM, jean-pierre charras wrote:
> It works on a complex hierarchy.
>
> But I found an other serious (blocking) issue: if there is a not yet
> annotated component in a hierarchy (even a simple hierarchy) the
> annotation dialog is always opened when trying to enter or leave a sheet
> (regardless the highlight tool is activated or not).
>

I missed that one. It's definitely a blocking issue. Thanks for
testing this.

[Nox_firegalaxy (nox-7bitfaster)]

Well would it be okay if the dialog is opened if a net is selected for highlighting and unannotated components exists? Or should the highlighting be silently deactivated as long as an unannotated component exists?

[jean-pierre charras (jp-charras)]

Sorry to say that, but neither are good (and neither are acceptable).

Unannotated components are not an issue to highlighting a connection.
I means the reference is not really used, although it cannot be fully ignored.

In fact you have 2 issues, when calling prepareForNetlist() (that is currently designed to prepare a netlist for a board):
- it cleanup the schematic (merge collinear wires, and therefore change the wire list).
- it tests everything strange thing in annotation.

Cleanup the schematic avoid side effects in connection calculation:
- It avoid false connections (This is a rare case, not very important here). But it has a side effect: if the wire list is modified, the undo/redo list is cleared (is has no matter when creating a netlist for a board, but it is is annoying in this case)
It could be avoided here.
- It calls CheckAnnotate().
CheckAnnotate() tests incorrect annotation (duplicates, not annotated and a few other things)
As I say previously, not annotated components are not blocking (other issues are, but they are very infrequent).
However annotating them before the schematic is finished is very annoying (think to not interchangeable units for instance).
Moreover, annotating components clears the undo/redo list.

Therefore:
* prepareForNetlist(), when called to highlight a net, should not cleanup the schematic and should no test for not annotated items.
* Not annotated items (only to calculate the connection list) should receive a temporary annotation (for instance the full timestamp).
The reason is the fact if you have 2 different components having the same ref (say R?) and the same pin number (say 1) all connection to pins (R? pin 1) will be merged.

[Nox_firegalaxy (nox-7bitfaster)]

After looking some time again:
One the one hand you are completely right. Neither killing the undo/redo
list nor the need to annotate everything is something one wants for
"simple" highlighting. On the other hand using the exact netlist export
process garantees that the highlighting is consistent with the exported
netlist. So is introducing a parallel procedure with temporary
annotation (which hopefully does not break something else) and avoidence
of the cleanup really the best option? What if someone changes the
behaviour only in one of both procedures?

So imho if one wants to get it right one should change the netlist
export to respect the undo/redo list - or is it good pratice that
export/sync breaks the undo/redo?
If I am not mistaken clearing the undo/redo list is done in (all
eeschema files listed):
LIB_EDIT_FRAME::OnImportPart
LIB_EDIT_FRAME::LoadComponentFromCurrentLib
LIB_EDIT_FRAME::CreateNewLibraryPart
SCH_SCREEN::~SCH_SCREEN
DIALOG_ERC::TestErc
SCH_SCREENS::SchematicCleanUp

For the here discussed case only the later three are of interested.
Interestingly SCH_SCREEN::SchematicCleanUp is called several times
without a correlated ClearUndoRedoList (e.g. AddNoConnect, EndSegment).
So my question is: Is ClearUndoRedoList really required/reasonable?
Regarding the annotation it would be better if it would not be required
but again: to avoid double doing and later divergence in behaviour I
would argue that it is the lesser evil to make annotation mandetory for
net highlighting - the user is still free to not use net highlighting at
all. Maybe I am naive but from a users perspective not having net
highlighting at all seems to me like a bigger issue than dealing with
annotation and even more threating is a net highlighting which is
potentially not consistent to netlist export.

P.S: Annotiation messes with undo/redo but neither clears the list nor
does it track changes if i am not mistaken. Just apply some changes
(e.g. rename parts), revert some changes, then run annotiation and play
around with undo/redo.

best regards

Am 01.11.2016 um 13:06 schrieb jean-pierre charras:
> Sorry to say that, but neither are good (and neither are acceptable).
>
> Unannotated components are not an issue to highlighting a connection.
> I means the reference is not really used, although it cannot be fully ignored.
>
> In fact you have 2 issues, when calling prepareForNetlist() (that is currently designed to prepare a netlist for a board):
> - it cleanup the schematic (merge collinear wires, and therefore change the wire list).
> - it tests everything strange thing in annotation.
>
> Cleanup the schematic avoid side effects in connection calculation:
> - It avoid false connections (This is a rare case, not very important here). But it has a side effect: if the wire list is modified, the undo/redo list is cleared (is has no matter when creating a netlist for a board, but it is is annoying in this case)
> It could be avoided here.
> - It calls CheckAnnotate().
> CheckAnnotate() tests incorrect annotation (duplicates, not annotated and a few other things)
> As I say previously, not annotated components are not blocking (other issues are,...

[Wayne Stambaugh (stambaughw)]

On 11/6/2016 7:27 AM, Nox_firegalaxy wrote:
> After looking some time again:
> One the one hand you are completely right. Neither killing the undo/redo
> list nor the need to annotate everything is something one wants for
> "simple" highlighting. On the other hand using the exact netlist export
> process garantees that the highlighting is consistent with the exported
> netlist. So is introducing a parallel procedure with temporary
> annotation (which hopefully does not break something else) and avoidence
> of the cleanup really the best option? What if someone changes the
> behaviour only in one of both procedures?

You're finally understanding why this hasn't been implemented yet. It
turns out that is a lot more difficult than it appears.

>
> So imho if one wants to get it right one should change the netlist
> export to respect the undo/redo list - or is it good pratice that
> export/sync breaks the undo/redo?
> If I am not mistaken clearing the undo/redo list is done in (all
> eeschema files listed):
> LIB_EDIT_FRAME::OnImportPart
> LIB_EDIT_FRAME::LoadComponentFromCurrentLib
> LIB_EDIT_FRAME::CreateNewLibraryPart
> SCH_SCREEN::~SCH_SCREEN
> DIALOG_ERC::TestErc
> SCH_SCREENS::SchematicCleanUp

SchematicCleanUp() is going to be an issue. The reason it clears the
undo/redo buffer is there would be dangling wire pointers if it didn't.
This behavior could be fixed. There are several options none of which
are going to be trivial to implement. Rather than clear the undo/redo
buffer, you could create a complete wire list a push that to the undo
buffer. The problem with that is when the user attempts to undo the
changes done by SchematicCleanUp() it will appear that nothing was
undone. You could append the changes to a previous undo buffer wire
list assuming there is one but that could be tricky. The other option
would be to call SchematicCleanUp() on every wire or junction change and
add any clean up changes to the undo buffer along with the wire or
junction changes.

>
> For the here discussed case only the later three are of interested.
> Interestingly SCH_SCREEN::SchematicCleanUp is called several times
> without a correlated ClearUndoRedoList (e.g. AddNoConnect, EndSegment).
> So my question is: Is ClearUndoRedoList really required/reasonable?
> Regarding the annotation it would be better if it would not be required
> but again: to avoid double doing and later divergence in behaviour I
> would argue that it is the lesser evil to make annotation mandetory for
> net highlighting - the user is still free to not use net highlighting at
> all. Maybe I am naive but from a users perspective not having net
> highlighting at all seems to me like a bigger issue than dealing with
> annotation and even more threating is a net highlighting which is
> potentially not consistent to netlist export.

Annotation is going to be an issue as well. The cleanest way to do it
is to make a temporary copy of the entire schematic and perform a full
reannotation. This way you would be ensured a valid netlist without
interfering with the users current annotation. I don't know what kind
of overhead would be incurred or if there would be eno...

[Nox_firegalaxy (nox-7bitfaster)]

I beg your pardon but actually it would not be that hard if the current
netlist export procedure would not mess up the undo/redo - which I would
expect as an user.

But it is how it is. So is there any good reason against your proposed
idea of cleaning up the wire right after insertion? By the way I am
curious why the undo/redo is only cleared some times.
SCH_SCREEN::SchematicCleanUp is called several times without a
correlated clearing of the redo/undo list (e.g. EndSegment,
AddNoConnect, DisplayCurrentSheet). Are the no dangling pointers in
those cases? Or is clearing the redo/undo list actually outdated?

And would writing an own netlist generator not mean again the
introduction of a parallel procedure?

Am 07.11.2016 um 16:48 schrieb Wayne Stambaugh:

> On 11/6/2016 7:27 AM, Nox_firegalaxy wrote:
>> After looking some time again:
>> One the one hand you are completely right. Neither killing the undo/redo
>> list nor the need to annotate everything is something one wants for
>> "simple" highlighting. On the other hand using the exact netlist export
>> process garantees that the highlighting is consistent with the exported
>> netlist. So is introducing a parallel procedure with temporary
>> annotation (which hopefully does not break something else) and avoidence
>> of the cleanup really the best option? What if someone changes the
>> behaviour only in one of both procedures?
> You're finally understanding why this hasn't been implemented yet. It
> turns out that is a lot more difficult than it appears.
>
>> So imho if one wants to get it right one should change the netlist
>> export to respect the undo/redo list - or is it good pratice that
>> export/sync breaks the undo/redo?
>> If I am not mistaken clearing the undo/redo list is done in (all
>> eeschema files listed):
>> LIB_EDIT_FRAME::OnImportPart
>> LIB_EDIT_FRAME::LoadComponentFromCurrentLib
>> LIB_EDIT_FRAME::CreateNewLibraryPart
>> SCH_SCREEN::~SCH_SCREEN
>> DIALOG_ERC::TestErc
>> SCH_SCREENS::SchematicCleanUp
> SchematicCleanUp() is going to be an issue. The reason it clears the
> undo/redo buffer is there would be dangling wire pointers if it didn't.
> This behavior could be fixed. There are several options none of which
> are going to be trivial to implement. Rather than clear the undo/redo
> buffer, you could create a complete wire list a push that to the undo
> buffer. The problem with that is when the user attempts to undo the
> changes done by SchematicCleanUp() it will appear that nothing was
> undone. You could append the changes to a previous undo buffer wire
> list assuming there is one but that could be tricky. The other option
> would be to call SchematicCleanUp() on every wire or junction change and
> add any clean up changes to the undo buffer along with the wire or
> junction changes.
>
>> For the here discussed case only the later three are of interested.
>> Interestingly SCH_SCREEN::SchematicCleanUp is called several times
>> without a correlated ClearUndoRedoList (e.g. AddNoConnect, EndSegment).
>> So my question is: Is ClearUndoRedoList really required/reasonable?
>> Regarding the annotation it would be better if it would not be required
>> but a...

[Wayne Stambaugh (stambaughw)]

On 11/7/2016 12:42 PM, Nox_firegalaxy wrote:
> I beg your pardon but actually it would not be that hard if the current
> netlist export procedure would not mess up the undo/redo - which I would
> expect as an user.
>
> But it is how it is. So is there any good reason against your proposed
> idea of cleaning up the wire right after insertion? By the way I am
> curious why the undo/redo is only cleared some times.
> SCH_SCREEN::SchematicCleanUp is called several times without a
> correlated clearing of the redo/undo list (e.g. EndSegment,
> AddNoConnect, DisplayCurrentSheet). Are the no dangling pointers in
> those cases? Or is clearing the redo/undo list actually outdated?

I'm OK with cleaning up immediately after each wire or junction change
but it will require some careful coding to make sure the undo/redo
buffers are correct for wire lists. Fixing this first would eliminate
the need to call SchematicCleanUp() before exporting the netlist or any
other operation that requires a cleaned up wire list. I would help you
with this if I had time but I am desperately trying to get the new file
format done.

The undo/redo buffers are only cleared when SchematicCleanUp() makes any
changes to the wiring. This is the case that causes the pointer issues
so it was an easy fix to prevent undoing bad wire pointers from causing
a segfault. It may be as simple as calling SchematicCleanUp() before
saving wire and junction edits to the undo buffer.

>
> And would writing an own netlist generator not mean again the
> introduction of a parallel procedure?

Maybe. It might be possible to extend the current netlist generator to
use symbol time stamps instead of reference designators for net
highlighting purposes. It seems like a better solution given all of the
issue with annotation. Users aren't going to want dismiss an annotation
warning dialog when they want to highlight a net. JP is much more
familiar with the netlist generation code than I am. He may have some
ideas that I have overlooked.

>
>
> Am 07.11.2016 um 16:48 schrieb Wayne Stambaugh:
>
>> On 11/6/2016 7:27 AM, Nox_firegalaxy wrote:
>>> After looking some time again:
>>> One the one hand you are completely right. Neither killing the undo/redo
>>> list nor the need to annotate everything is something one wants for
>>> "simple" highlighting. On the other hand using the exact netlist export
>>> process garantees that the highlighting is consistent with the exported
>>> netlist. So is introducing a parallel procedure with temporary
>>> annotation (which hopefully does not break something else) and avoidence
>>> of the cleanup really the best option? What if someone changes the
>>> behaviour only in one of both procedures?
>> You're finally understanding why this hasn't been implemented yet. It
>> turns out that is a lot more difficult than it appears.
>>
>>> So imho if one wants to get it right one should change the netlist
>>> export to respect the undo/redo list - or is it good pratice that
>>> export/sync breaks the undo/redo?
>>> If I am not mistaken clearing the undo/redo list is done in (all
>>> eeschema files listed):
>>> LIB_EDIT_FRAME::OnImportPart
>>> LIB_EDIT_FRAME::Loa...

[jean-pierre charras (jp-charras)]

In DisplayCurrentSheet, SchematicCleanUp is called only once, the first time the sch_screen is displayed, so the undo list is empty.

EndSegment saves the old wire list in undo list before calling SchematicCleanUp.

Calling SchematicCleanUp in AddNoConnect without saving the old wiring is suspicious. Could be a bug.

Calling SchematicCleanUp before building the netlist is mainly made to avoid very rare corner cases taht can create unexpected connections.
From my point of view, calling SchematicCleanUp is not mandatory to highlight a connection.
(Remenber SchematicCleanUp is called when editing wires, so it is frequently called on connection change)

There is still the case of not yet annotated components.
This is the major issue.
One cannot annotate new components during the creation of a schematic at any time without creating serious issues (Especially with multiple units per package components and/or inside a hierarchy)
Most of time the annotation will be no suitable (for instance a not wanted selection of units)
For me, the best way is to *temporary* use (only for highlight purpose) the full time stamp as replacement of the the standard reference for these components (including multiple units per package components, without changing the unit selection)
Perhaps (just an idea) this temporary reference could be build and stored in the pin NETLIST_OBJECT (for highlight purpose only).

I don't think we can always easily use the time stamp instead of the reference to build a netlist (especially fo pcbnew) because for multiple units per package components there are many time stamps (one by unit) for only one reference (one footprint).
And grouping the pins by package can be made only when the reference is known.

[Wayne Stambaugh (stambaughw)]

On 11/8/2016 2:43 AM, jean-pierre charras wrote:
> In DisplayCurrentSheet, SchematicCleanUp is called only once, the first
> time the sch_screen is displayed, so the undo list is empty.
>
> EndSegment saves the old wire list in undo list before calling
> SchematicCleanUp.
>
> Calling SchematicCleanUp in AddNoConnect without saving the old wiring
> is suspicious. Could be a bug.

The only time the clear undo/redo list is called is when
SCH_SCREENS::SchematicCleanUp() is called to clean up all of the
schematics. SCH_SCREEN::SchematicCleanUp() does not clear the undo/redo
list so it possible that only the current sheet needs to be cleaned up
before highlighting the net.

>
> Calling SchematicCleanUp before building the netlist is mainly made to avoid very rare corner cases taht can create unexpected connections.
>>From my point of view, calling SchematicCleanUp is not mandatory to highlight a connection.
> (Remenber SchematicCleanUp is called when editing wires, so it is frequently called on connection change)
>
> There is still the case of not yet annotated components.
> This is the major issue.
> One cannot annotate new components during the creation of a schematic at any time without creating serious issues (Especially with multiple units per package components and/or inside a hierarchy)
> Most of time the annotation will be no suitable (for instance a not wanted selection of units)
> For me, the best way is to *temporary* use (only for highlight purpose) the full time stamp as replacement of the the standard reference for these components (including multiple units per package components, without changing the unit selection)
> Perhaps (just an idea) this temporary reference could be build and stored in the pin NETLIST_OBJECT (for highlight purpose only).
>
> I don't think we can always easily use the time stamp instead of the reference to build a netlist (especially fo pcbnew) because for multiple units per package components there are many time stamps (one by unit) for only one reference (one footprint).
> And grouping the pins by package can be made only when the reference is known.

I was thinking that using time stamps to create the netlist would only
be used for net highlighting. The current netlist should be used for
all other purposes. The time stamp may be an alternative to using
temporary annotation. Displaying an annotation warning every time there
annotation conflict just to highlight a net would really be annoying.
Having the annotation changed without the user's input would be even
worse. We have to figure out a way to identify unique symbols in the
netlist without touching the existing annotation.

>

[Nox_firegalaxy (nox-7bitfaster)]

Thank you for the very valuable information. You are right that calling SchematicCleanUp is not necessary for highlighting. None the less it is annoying that an export kills und/redo capability. So actually this is a seperate issue. Maybe we should split this topic off. Would it be feastable to make SCH_SCREEN::Remove generally undo aware and introduce a "batching" for changes in the undo/redo? This way one might avoid potential bugs like the supposed bug in AddNoConnect. Or is there a case that some item has to be removed without being "logged"?

I looked into the proposed approaches (using timestamp/temporary annotation) and tried to find all access made by netlist generation to annotations. If I am not mistaken annotations are only accessed in GetShortNetName. The netlist algorithm itself first looks up all physical connections, takes care of the label connections and finally cleans up a bit. So only the label step make use of the names and as labels are higher priority, annotations should never affect the result of the connection algorithm, or did I miss something?

Under these assumptions version 08 should work without complete annotation and without cleaning up. Remaining issues are the warp after the first selection and the unresolved short cut semantic.

[jean-pierre charras (jp-charras)]

This patch looks good now.
I slightly modified you patch to fix these remaining issues (hot key is Ctrl X)
Here is the patch.
Try it.

[Nox_firegalaxy (nox-7bitfaster)]

Works fine here!
minor suggestions:

eeschema/schframe.h:462 HighlightConnectionAtCrossHair should be HighlightConnectionAtPosition.
Maybe move HighlightConnectionAtPosition from eeschema/onleftclick.cpp to eeschema/schframe.cpp?

[jean-pierre charras (jp-charras)]

You are right.
I fixed that, and committed your contribution, with my latest fixes.
Thanks for your work.

[Reece Pollack (reece-pollack)]

I love the new "Highlight Net" feature (testing with 5.0.0-rc2).

I realize this may be too late for the 5.0.0 release, but would it be possible to make the color of the connected *pins* change change as well as the wire? This would allow the user to see at an instant if a wire really connects with a pin. Right now I resort to grabbing the device symbol and dragging it around to see if everything is connected.

We can put this on the wish list for a future release if necessary.

[Seth Hillbrand (sethh)]

@reece-pollack Please open a new bug report for this wishlist item. This seems feasible for v6.

On the connected or not issue, you should always see the open square at a wire end/pin end when they are not connected.