Allow pin labels in connector loop definitions (fixes #432)#493
Draft
rsp2k wants to merge 4 commits intowireviz:masterfrom
Draft
Allow pin labels in connector loop definitions (fixes #432)#493rsp2k wants to merge 4 commits intowireviz:masterfrom
rsp2k wants to merge 4 commits intowireviz:masterfrom
Conversation
Loops now accept pin labels (from pinlabels) in addition to pin numbers, matching the behavior of the connections section. Labels are resolved to pin numbers during __post_init__ via the new Connector.resolve_pin() method, which handles ambiguity checking.
Code review fixes for the wireviz#432 loop-pin-labels feature: - Fix loop rendering to use port indices instead of pin numbers (pre-existing bug: non-sequential pins produced wrong diagram) - Add duplicate label check to the ambiguity branch in resolve_pin() - Prevent self-referencing loops (pin looped to itself) - Fix activate_pin() type annotation to accept Optional[Side] - Deduplicate pin resolution: Harness.connect() now delegates to Connector.resolve_pin() instead of reimplementing the logic - Add 21-test suite covering all resolution paths and error modes
- Document return contract, resolution precedence, and type-sensitivity note in resolve_pin() docstring - Add ex17_loop_labels.yml: RS-232 loopback adapter demonstrating loops with pin labels, mixed number+label, and label-based connections
YAML safe_load() parses unquoted 1 as int and quoted "1" as str. Python's `in` and .index() are type-strict, so pin lookups silently fail when users quote numeric pin values. Extract normalize_pin() from expand()'s inline logic and call it on pins, pinlabels, loops, and wirelabels in __post_init__() before any validation or resolution. Downstream code sees consistent types.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Connector
loopsnow accept pin labels (frompinlabels) in addition to pin numbers, matching how regular connections already work.Also fixes two pre-existing bugs:
pins: [1]vspins: ["1"]) caused silent pin lookup failuresChanges
wv_helper.pynormalize_pin()function — triesint()first, falls back tostr(). Extracts the coercion logic that was already inline inexpand().expand()refactored — usesnormalize_pin()instead of inline try/except.DataClasses.pyConnector.resolve_pin()method — resolves a pin identifier (number or label) to its canonical pin number. Handles ambiguity detection (value in bothpinsandpinlabelsat different positions) and duplicate label detection.Connector.__post_init__()—pins,pinlabels, andloopsare normalized vianormalize_pin()before any validation. This ensurespins: ["1", "2"]andpins: [1, 2]behave identically regardless of YAML quoting.Cable.__post_init__()—wirelabelsnormalized the same way.resolve_pin()and store the resolved pin numbers. Self-loops (same pin on both ends) are rejected.activate_pin()type annotation —sideparameter updated toOptional[Side]since loops passNone.Harness.pyconnect()refactored — pin resolution logic (previously ~20 lines duplicating the same algorithm) replaced with delegation toConnector.resolve_pin(). Behavior is unchanged, but the logic now lives in one place.connector.pins.index(), matching how wire connections already work. Previously, pin numbers were used directly as port names, which produced silently wrong diagrams for non-sequential pins likepins: [10, 20, 30, 40].examples/ex17_loop_labels.yml(new)RS-232 loopback adapter example demonstrating loops with pin labels, mixed number + label, and label-based wire connections.
tests/test_resolve_pin.py(new)31 tests covering:
resolve_pin()happy paths and error pathsTest plan
pins: ["1", "2", "3"]withloops: [[1, "2"]]resolves correctly"A1","3.3V")Notes
loopsaccepts the same types as before, plus strings that matchpinlabels. Existing YAML files are unaffected by pin normalization.