Skip to content

refactor(examples): parameterize browse_tags.py, drop unittest.mock hack#767

Merged
gijzelaerr merged 1 commit into
gijzelaerr:masterfrom
tommasofaedo:cleanup/browse-tags-argparse
Jul 10, 2026
Merged

refactor(examples): parameterize browse_tags.py, drop unittest.mock hack#767
gijzelaerr merged 1 commit into
gijzelaerr:masterfrom
tommasofaedo:cleanup/browse-tags-argparse

Conversation

@tommasofaedo

Copy link
Copy Markdown
Contributor

Follow-up cleanup for examples/browse_tags.py (merged in #756).

Why

In your #756 review you flagged two blockers — the hardcoded PLC IP and the unittest.mock hack — and later said you'd pushed fixes for them. That cleanup commit didn't make it into the merge (#756 landed with commits: 1), so examples/browse_tags.py in master still has both. This PR applies them for real, and folds in a small correctness fix.

Changes

  • argparse CLI — positional host, optional area letters, --port, --tls, --password. No more module-level PLC_HOST = "192.168.5.11".
    python browse_tags.py 192.168.5.11            # all areas, plaintext
    python browse_tags.py 192.168.5.11 --tls      # TLS-only PLCs (FW V4.1 "G2")
    python browse_tags.py 10.0.0.5 --tls I Q      # subset of areas
    
  • Remove unittest.mock — the old code wrapped connect() in mock.patch.object(S7CommPlusConnection, "_post_auth_legitimation", ...). connect() already wraps the optional post-auth legitimation in a try/except that only logs a warning when it's skipped (no-password PLCs, or the plaintext V3 path where legitimation needs TLS), so the mock was redundant. Verified the browse works unchanged without it.
  • I-area address fix — the 43 in the FDICT sample %I43.x is not the real tag address (only the bit digit is a literal); rebuild %I{ByteOffset}.{bit} so input_1 resolves to %I0.0 instead of %I43.0.
  • PoC disclaimer in the module docstring (FDICT is firmware-family specific).

Tested on

Re-validated on a S7-1200 CPU 1212C, FW V4.1 ("G2"), over TLS 1.3 (mandatory on that unit — the FW V4.5 unit from #756 is no longer available to me, see #753):

$ python browse_tags.py <host> --tls
>> Area I  (RID=80) ... input_1   Bool  %I0.0  0  10
>> Area Q  (RID=81) ... output_1  Bool  %Q0.1  0   9
>> Area M  (RID=82) ... merker_1  Bool  %M0.2  0   9

All three project tags resolve to their exact TIA Portal addresses.

ruff format / ruff check clean. Example-only — no library code touched.

…nittest.mock

Follow-up cleanup for examples/browse_tags.py (merged in gijzelaerr#756). The two
review points the maintainer raised — hardcoded PLC IP and the
`unittest.mock` hack — were not present in the merged commit; this applies
them, plus a fix that makes the I-area address resolve correctly.

- argparse CLI: positional `host`, optional area letters, `--port`,
  `--tls`, `--password`. No more module-level `PLC_HOST = "192.168.5.11"`.
- Remove `from unittest import mock` and the
  `mock.patch.object(..., "_post_auth_legitimation", ...)` wrapper.
  connect() already wraps the optional post-auth legitimation in a
  try/except and only logs a warning when it is skipped (no-password PLCs,
  or the plaintext V3 path where legitimation needs TLS), so the mock was
  redundant. Verified: browse works unchanged without it.
- I-area LogicalAddress fix: the '43' in the FDICT sample '%I43.x' is not
  the real tag address; only the bit digit is literal. Rebuild
  %I{ByteOffset}.{bit} so `input_1` resolves to %I0.0 instead of %I43.0.
- PoC disclaimer in the module docstring.

Tested on an S7-1200 CPU 1212C, FW V4.1 ("G2"), over TLS 1.3 (mandatory on
that unit): `python browse_tags.py <host> --tls` resolves all three project
tags exactly — input_1=%I0.0, output_1=%Q0.1, merker_1=%M0.2.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5LBhHS3Xm2Khj5N974kPr

@gijzelaerr gijzelaerr left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! sorry merged that too quickly, you right

@gijzelaerr gijzelaerr merged commit b6d8e97 into gijzelaerr:master Jul 10, 2026
36 checks passed
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.

2 participants