Skip to content

Reject unsupported schematic properties without stopping the Altium bridge - #35

Closed
TerrenceHenry wants to merge 2 commits into
salitronic:mainfrom
TerrenceHenry:fix/altium-property-access-crashes
Closed

TerrenceHenry wants to merge 2 commits into
salitronic:mainfrom
TerrenceHenry:fix/altium-property-access-crashes

Conversation

@TerrenceHenry

@TerrenceHenry TerrenceHenry commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

On Altium 21, creating an eNetLabel with IsHidden or querying an eParameterSet's Text opened an undeclared-identifier dialog and stopped bridge polling. Guard both reads and writes before member access. Reject unsupported create items before registration, dispose of the temporary object, and report the property/type while allowing valid batch items to continue.

Closes #34. Related to the already-closed #22.

The change preserves tool inputs and supported property paths. Batch failures add a property field with reason UNSUPPORTED_PROPERTY. It follows the existing narrow capability guards rather than introducing a complete property registry. No customer design is included in the patch.

Type and areas

  • Bug fix
  • DelphiScript bridge
  • Tests and CI
  • Documentation (including the Python tool description)

Validation

  • DelphiScript lint: 0 errors, 0 warnings.
  • Pascal bundle generation.
  • Focused offline checks: 33 passed, 23 explicitly skipped because Free Pascal is absent locally.
  • In-memory mutation checks detect removal of each read/write guard and rejected-type exclusion.
  • Full offline suite: 5,215 passed, 146 skipped, 1 xfailed, and 5 temporary-directory permission failures. All 5 passed on a focused rerun with PYTEST_DEBUG_TEMPROOT pointed at a fresh isolated directory. No source changes were needed. Command: python -m pytest --ignore=tests/integration -n 4 -q -rs. Skips cover absent FPC, optional vendor references/build artifacts, and local binary fixtures.
  • Pascal execution: new tests extract the production guards/parser and create routines and execute them against minimal observable mocks. The CI job with FPC now includes these tests; local compiler-dependent cases were skipped. GitHub run 36630877711 is action_required with no jobs executed, so CI/FPC validation remains pending maintainer action.
  • Live Altium 21.4.1.30 validation passed on a disposable schematic after a full application restart. The deployed Generic.pas matched commit 89c017f (SHA-256 36f9c0dde95de366a1b0ea60bc653793ed6f915c1c7714d08ee20bb791aef57e). Unsupported reads and single/batch writes returned diagnostics; both single creates left counts unchanged; mixed invalid/valid/invalid/valid creation returned exactly 2 created and 2 indexed failures. Supported parameter IsHidden read back false/true/false. Ping and a valid query completed after each rejection, without a modal error or polling restart. Recorded-response assertions passed.

Review notes

Live Altium acceptance is complete. Keep this PR in draft while upstream CI/FPC execution is gated by action_required. Restart Altium after installing candidate scripts: it caches Pascal scripts. The disposable-schematic procedure is in docs/RELEASE_VERIFICATION.md and includes object counts, invalid/valid mixed batches, and ping/query continuity after rejection.

Commits use plain imperative subjects per CONTRIBUTING.md (the template's conventional-commit checkbox is stale). No secrets, customer designs, proprietary library paths, generated bundles or version bump are included. Stale compiled connectivity remains out of scope.

Validation note: for future acceptance runs, close unrelated design documents or retain byte-for-byte backups first. Save-all can rewrite already-clean open sheets; it is not a preservation check.

@salitronic

Copy link
Copy Markdown
Owner

Thanks, this is a good fix. Tested on AD 26.10.1.6: both cases from #34 reproduce there, and IsHidden on a wire stops the bridge the same way, so excluding only net labels isn't enough. I'll take your commits and turn the checks into an allow-list of the types that actually have the property.

@salitronic

Copy link
Copy Markdown
Owner

Merged by cherry-pick as e485be2 and ce83416, with your authorship kept. On top, 57760ac makes the IsHidden check an allowlist and extends the Text denylist, after the wire case also stopped the bridge on AD 26. Thanks!

@salitronic salitronic closed this Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants