flandreas / antares

Digital circuit learning platform
49 stars 6 forks source link

Adding ports to connected components can change connections #704

Closed fpw closed 7 months ago

fpw commented 7 months ago

This one happened a few times to me and I think it's quite severe: When new ports are added to a sub-circuit and the circuit is already used and connected somewhere, it's possible that the existing connections suddenly become wrong, sometimes even nearly invisibly. I haven't figured out when exactly this happens, but here's one example that seems to be always reproducible (it actually shows two bugs because the same thing happens when deleting ports):

https://github.com/flandreas/antares/assets/5798899/c644e278-3692-4450-815d-15ec57887ecc

flandreas commented 7 months ago

@fpw The reason why this happens is that Antares doesn't store the port names when connecting wires to subcircuit pins. This is because it would break when renaming pins. Instead, it uses the internal index of the port, such as 1, 2, 3 and so forth.

Now, until a couple of releases ago, this principle worked fine even when adding additional ports to a subcircuit. Because the user had to manually add these ports to the symbol, the additional ports received higher port numbers, and the existing one were untouched.

This changed when we introduced the "Generate Symbol" feature. A symbol is automatically laid-out again whenever a port is added or removed. The algorithm first removes all ports, then adds all inputs to the left side and all outputs to the right side of the symbol, leading to new port IDs. This is why in your example, the LED, which was previously connected with former port 2 (the O1 output), is now connected to I2, because that is now the port 2 (because it is the second port after the relayout.

I have to think about how to change the layout algorithms to make the port assignments more robust with these use cases.

fpw commented 7 months ago

Thanks for the explanation of the internals! Not sure if it helps, but my first idea would be this:

flandreas commented 7 months ago

@fpw Yes that helps, I'll do it along these lines.