Skip to content

Add runtime typed ADS type system with self-contained tests#516

Open
Rock4Fun wants to merge 1 commit into
stlehmann:masterfrom
Rock4Fun:typed-ads-upstream-ready
Open

Add runtime typed ADS type system with self-contained tests#516
Rock4Fun wants to merge 1 commit into
stlehmann:masterfrom
Rock4Fun:typed-ads-upstream-ready

Conversation

@Rock4Fun
Copy link
Copy Markdown

@Rock4Fun Rock4Fun commented May 5, 2026

Summary

This PR adds a read-only runtime typed ADS layer that can build a type system from ADS symbol and datatype uploads and decode complex PLC values without requiring hand-written structure_def.

It introduces a new TypeSystem API and integrates it into Connection, with backward-compatible aliasing.

What is included

  • New module: src/pyads/typed_symbols.py

    • Upload helpers for symbol/datatype blobs (ADSIGRP_SYM_UPLOAD, ADSIGRP_SYM_UPLOADINFO2, fallback to ADSIGRP_SYM_UPLOADINFO, ADSIGRP_SYM_DT_UPLOAD)
    • Symbol/datatype parsers
    • Runtime type lookup maps (type names, declarations, aliases)
    • Read-only decoder for:
      • Scalars
      • STRING / WSTRING
      • Structs / FBs via subitem offsets
      • Arrays (incl. multidimensional)
      • Pointer/reference values as opaque addresses (no dereference)
      • Unknown/recursive/incomplete/hidden types as opaque raw payloads
    • Batch read strategy (read_values(..., batch=True)) with root grouping and sum-read support
  • Connection API additions (src/pyads/connection.py)

    • get_type_system(refresh=False, debug=False)
    • get_typed_symbol_table() (compatibility alias)
  • Public exports (src/pyads/__init__.py)

    • TypeSystem
    • TypedSymbolTable
  • Testserver improvements

    • src/pyads/testserver/advanced_handler.py
      • fixture-style symbol/datatype upload responses for:
        • SYM_UPLOADINFO2
        • SYM_UPLOADINFO
        • SYM_UPLOAD
        • SYM_DT_UPLOAD
    • src/pyads/testserver/testserver.py
      • configurable port via PYADS_TESTSERVER_PORT
      • clearer bind errors
  • Tests

    • tests/test_typed_symbols.py (self-contained, no binary fixture files required)
    • tests/test_typed_symbols_live.py (optional live tests, gated by PYADS_RUN_LIVE=1)
    • Existing tests hardened to skip cleanly when local fixed ports are unavailable:
      • tests/test_connection_class.py
      • tests/test_symbol.py
      • tests/test_testserver.py
      • tests/test_plc_ams.py
      • tests/test_plc_route.py
    • Local config template:
      • tests/live_ads_config.example.json
    • Local live config ignored:
      • .gitignore adds tests/live_ads_config.json

Validation

Executed locally:

  • PYTHONPATH=src python -m pytest tests/test_typed_symbols.py tests/test_typed_symbols_live.py -q
    • 9 passed, 4 skipped
  • PYTHONPATH=src PYADS_RUN_LIVE=1 python -m pytest tests/test_typed_symbols_live.py -q
    • 2 passed

(plus targeted testserver/UDP-related tests with expected skip behavior on blocked local ports)

Notes

  • Scope is intentionally read-only typed ADS support.
  • Typed write support and higher-level integrations (e.g. Home Assistant discovery/config workflows) are out of scope for this PR.

@chrisbeardy
Copy link
Copy Markdown
Collaborator

Hi , I'm not against using AI for coding, but I would appreciate that if delivering a PR on something this size and significant feature change for this library that a more human written PR and or discussion is also created, as currently this PR has a few red flags for me. I appreciate that these may just be red flags and actually there is real intent behind this PR but if possible could you explain why you have done this, the purpose and the the root problem you were trying to solve. I think for a feature of this size the maintainers need to discuss its inclusion and them maintenance burden. Currently we have just recieved a AI written PR and code which looks completely AI generated, again I am not against that but want to determine the level of vetting required.

@Rock4Fun
Copy link
Copy Markdown
Author

Rock4Fun commented May 6, 2026

Hi, thanks for the honest feedback. I understand the concern.

The background is very concrete: I have a Beckhoff PLC in my house which controls shutters, heating, wind sensors and several other parts of the building automation. I want to integrate this cleanly into Home Assistant.
The root problem I am trying to solve is that pyads currently requires a lot of manual structure knowledge when reading complex PLC data. For my use case I would like Home Assistant to be able to scan the PLC, read the available symbols and datatype information, understand structs/arrays/nested values, and then allow me to select which values should be exposed in dashboards. I do not want to manually maintain matching Python structure definitions for every PLC-side change.
I started working on this idea some time ago with local experiments and binary dumps from my PLC, then picked it up again recently. During the recent work I used Codex and GPT-5.5 heavily to help with implementation, tests and cleanup. I should have made that clearer and also should have written a more personal PR description explaining the actual motivation and scope.
Sorry that the PR came across as too impersonal and AI-generated. That was not my intention. There is a real use case behind this, and I have tested it against my own Beckhoff setup.
I completely understand that this is a significant feature and may create maintenance burden. I would be happy to work with you to reduce the red flags, split the PR into smaller parts, improve the design discussion, add documentation, or narrow the initial scope to something easier to review.
My goal is not to dump generated code on the project, but to contribute a useful foundation for runtime symbol/datatype introspection in pyads.

@chrisbeardy
Copy link
Copy Markdown
Collaborator

Thank you for your reply, I understand. This is also a feature that we have wanted for a while so we don't have to manually keep structures up to date, I will look at the implementation to see how we best fit it in.

@stlehmann
Copy link
Copy Markdown
Owner

I like the idea.

I will look at the implementation to see how we best fit it in.

Seems like at least 90% of the changes concern testing. Focussing on the actual code changes could make things easier.

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.

3 participants