refactor(examples): parameterize browse_tags.py, drop unittest.mock hack#767
Merged
gijzelaerr merged 1 commit intoJul 10, 2026
Merged
Conversation
…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
approved these changes
Jul 10, 2026
gijzelaerr
left a comment
Owner
There was a problem hiding this comment.
thanks! sorry merged that too quickly, you right
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.
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.mockhack — and later said you'd pushed fixes for them. That cleanup commit didn't make it into the merge (#756 landed withcommits: 1), soexamples/browse_tags.pyinmasterstill has both. This PR applies them for real, and folds in a small correctness fix.Changes
host, optional area letters,--port,--tls,--password. No more module-levelPLC_HOST = "192.168.5.11".unittest.mock— the old code wrappedconnect()inmock.patch.object(S7CommPlusConnection, "_post_auth_legitimation", ...).connect()already wraps the optional post-auth legitimation in atry/exceptthat 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.43in the FDICT sample%I43.xis not the real tag address (only the bit digit is a literal); rebuild%I{ByteOffset}.{bit}soinput_1resolves to%I0.0instead of%I43.0.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):
All three project tags resolve to their exact TIA Portal addresses.
ruff format/ruff checkclean. Example-only — no library code touched.