Skip to content
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

Fixes dragging bendpoints of 4 or more wires (fixes #3006). #3942

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

failiz
Copy link
Contributor

@failiz failiz commented Mar 24, 2022

This fixes issue #3006. When dragging a bendpoint with a junction of 4 or more wires, all wires are updated correctly. This fixes the schematic and PCB views.
There is a similar problem (issue #3580) when dragging a part (the wires connected to that part are not updated correctly). Probably, it is the same cause as in this bug, but in a different part of the code (dragging part code). There is a video of this issue also in #3006.

…onnected). Fixes fritzing#3933 and fritzing#3827 Now, when an item is connected to the breadboard, only the holes of the breadboard that are connected are in green; a wire connecting two holes of the breadboard has both ends in green.
@KjellMorgenstern
Copy link
Member

The yellow highlight for same level subnet should still highlight all connectors of that breadboard strip. Is it affected by the patch?

@failiz
Copy link
Contributor Author

failiz commented Mar 28, 2022

The yellow highlight for same level subnet should still highlight all connectors of that breadboard strip. Is it affected by the patch?

No, that is handed by a different code and still works with my fix. My patch only modifies the colors of the connectors (green, red or no color) based on their connection status.

@failiz
Copy link
Contributor Author

failiz commented May 19, 2023

Hi,
I just noticed that the dragging of the bendpoints of 4 wires has been fixed in 1.0.0. However, the second commit (580a8c4) "Improvements to the connectors status color (red=unconnected, green=connected). Fixes #3933 and #3827 Now, when an item is connected to the breadboard, only the holes of the breadboard that are connected are in green; a wire connecting two holes of the breadboard has both ends in green." has not been merged. Any issues with the fix?

Edit: Notice that the commit fixes #3827

@KjellMorgenstern
Copy link
Member

It looks like with the patch applied, all connectors are marked "connected", even if none is:
image

…her in the PCB. Thus, we need to remove the opposite connector.
@failiz
Copy link
Contributor Author

failiz commented May 19, 2023

Yes, I tried it in the breadboard and schematic but I did not notice the error in the PCB view. The problem is that is finding two connectors (the top one and the bottom one). I have made a fix to consider only one of the connectors of the stack. Could you try it again to double-check it?

@failiz
Copy link
Contributor Author

failiz commented May 19, 2023

image

I think it is better than before but there is something that does not work correctly in PCB. If you create traces that are not connected to anything and move the end of the unconnected traces, these traces are in green:
image
But when you move the component, then they all get a red color:
image

…pad stack (top or bottom) to the list of connected connectors. The previous commit only worked if the function was called from a connector in a pad stack. If the connector was from a trace, then we were still adding the two connectors of the pad stack.
@failiz
Copy link
Contributor Author

failiz commented May 19, 2023

Ok. I fixed the issue. Can you give it a try?
I think the behavior is better than the current one.

@@ -1365,7 +1386,7 @@ void ConnectorItem::collectEqualPotential(QList<ConnectorItem *> &connectorItems
// When the kept connector item is part of a bus, include all of the other
// connectors on the bus in the list being processed
Bus *bus = connectorItem->bus();
if (bus) {
if (bus && (followBuses||fromWire)) {
QList<ConnectorItem *> busConnectedItems;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the "skipBuses" param was broken, or maybe never implemented.
For the IPC-D 356 export we also needed this and I modified this to

@@ -1381,7 +1384,7 @@ void ConnectorItem::collectEqualPotential(QList<ConnectorItem *> &connectorItems
                // When the kept connector item is part of a bus, include all of the other
                // connectors on the bus in the list being processed
                Bus *bus = connectorItem->bus();
-               if (bus) {
+               if (!skipBuses && bus) {
                        QList<ConnectorItem *> busConnectedItems;

The renaming is not an issue, but I am a bit paranoid about the "fromWire". The collectEqualPotential method is quite critical. Do you think by adding the skipBuses like in our current version, we are excluding to many items?

Is the patch about more than just highlighting the connections? Or do we need it to send proper netlists to ngspice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think skipBuses was never implemented and you implemented it recently. At least, I cannot find it in the source code. I guess skipBuses and followBuses are basically the same.

The fromWire is because I want to avoid following buses of the parts (e.g., breadboard or Arduino pins), but I still want to follow the wires. And the wires are implemented as buses. Removing the fromWire from the code breaks the connector highlighting.

The patch is to fix issue #3827 and is just about improving the highlighting of the connector status (not related to the SPICE netlist). That issue is caused because the current algorithm checks if a part is connected to another part (ItemBase). In my patch, I changed it to check if a connector is connected to other connectors (from the same part or from a different one). No following the buses makes that ony the hole of the breadboard that is connected is highlighted.

In my code, the only point where buses are not followed is in the restoreColor function. The code for generating the spice netlists uses the default argument of followBuses (as it is necessary to follow the buses). Not sure how the exporting of the IPC-D 356 netlists works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants