Comment 45 for bug 594073

Revision history for this message
Nox_firegalaxy (nox-7bitfaster) wrote : Re: [Bug 594073] Re: Highlight Nets

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