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

[bug] Pin case sensitivity. #160

Closed
zombielinux opened this issue Aug 28, 2020 · 4 comments
Closed

[bug] Pin case sensitivity. #160

zombielinux opened this issue Aug 28, 2020 · 4 comments
Labels
bug Something isn't working
Milestone

Comments

@zombielinux
Copy link
Contributor

I'm looking at implementing this but I use a lot of mil-circ connectors.

As such, they typically run A-Z, then a-z, then I think aa-zz.

As it stands, defining the template:

   - &MS3126F18-32P
     type: MS3126F18-32P
     pins: [A, B, C, D, E, F, G, H, J, K, L, M, N, P, R, S, T, U, V, W, X, Y, Z, a, b, c, d, e, f, g, h, j]

And then using pin "a" actually connects pin "A". In my case, these are VERY different pins. This is with the latest dev branch.

@kvid
Copy link
Collaborator

kvid commented Aug 29, 2020

Thank you for reporting this bug. It seems WireViz is case sensitive about identifiers, but unfortunately Graphviz is not. This also means that a similar identifier problem might occur when a connector/cable name is not case insensitive unique.

I suggest keeping WireViz case sensitive to allow use cases like yours, and use pin index instead of pin name in all connector port identifiers to ensure their uniqueness. If it is really important to keep the pin name in the port identifier, then it might be appended as a suffix to the pin index when different from it.

@formatc1702 formatc1702 changed the title Pin case sensitivity. [bug] Pin case sensitivity. Oct 16, 2020
@formatc1702 formatc1702 added this to the v0.3 milestone Oct 17, 2020
@formatc1702
Copy link
Collaborator

formatc1702 commented Oct 20, 2020

Here's a quick test that displays the observed behavior:

connectors:
  X1:
    pins: [AA, Aa, aA, aa]
  X2:
    pins: [ZZ, Zz, zZ, zz]

cables:
  W1:
    wirecount: 4
    color_code: DIN

connections:
  -
    - X1: [AA, Aa, aA, aa]
    - W1: [1-4]
    - X2: [zz, ZZ, Zz, zZ]

Expected result:
image

Actual result:
test

Workaround to display expected result:
Replace A with _A_ and Z with _Z_ in GraphViz file.
This is obviously not the solution to the underlying issue; just leaving this here for reference.

@formatc1702
Copy link
Collaborator

I suggest keeping WireViz case sensitive to allow use cases like yours, and use pin index instead of pin name in all connector port identifiers to ensure their uniqueness.

+1 👍

@formatc1702
Copy link
Collaborator

formatc1702 commented Nov 11, 2020

The safest approach is to use an internally assigned, numerical, auto-incremented ID to every pin (analogously, assign a similar ID to every wire in a cable to iterate over) and map the user-specified pin ID to this internal one.

In the typical case where a user does not specify a connector's pins directly, and they are thus auto-numbered, the internal ID and the "user-visible" ID used when defining a connection will simply be the same.

In the above mentioned case, Python/WireViz will case-sensitively match the user-provided pins to their respective unambiguous numerical IDs.

This also solves a semi-related issue:
When using pins with unusual characters (<,> and others), GraphViz is unable to draw the edges because the port names it uses to attach the edge to a node currently contain the pins attribute, causing issues with GraphViz' internal HTML when pins contains such illegal characters.

See #230 for this issue.

formatc1702 added a commit that referenced this issue Aug 23, 2021
Closes #160.

Co-authored-by: kvid <kvid@users.noreply.github.com>
formatc1702 added a commit that referenced this issue Aug 23, 2021
formatc1702 added a commit that referenced this issue Jun 7, 2023
@kvid kvid added the bug Something isn't working label Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants