fix(online tools): auto-login + non-CONSTANT version GVL
Two related v5-sweep fixes for the online/runtime tool family: 1. Auto-login helper for headless mode In headless mode each MCP call spawns a fresh CODESYS --noUI process, so the login state established by connect_to_device dies before the next call. Pre-fix, only connect_to_device and download_to_device did their own login(); the other four (start_stop_application, read_variable, write_variable, read_running_version_online) silently failed in headless with 'Application not logged in.' (start/stop) or 'Invalid expression' (read/write). They worked in persistent mode only because the login carried across calls. Added ensure_logged_in(online_app, login_wait_seconds=30) to ensure_online_connection.py. Idempotent: short-circuits via online_app.is_logged_in (persistent mode is a no-op, no extra login roundtrip). When not logged in, runs the same enum-probe + call-shape probe + STABLE_STATES settle-wait pattern as connect_to_device.py. Added to start_stop_application.py, read_variable.py, write_variable.py, read_running_version_online.py. 2. _MCP_PROJECT_VERSION GVL emitted as plain VAR_GLOBAL, not CONSTANT CODESYS inlines VAR_GLOBAL CONSTANT scalars at compile time and strips them from the online symbol table. The whole point of _MCP_PROJECT_VERSION.sVersion is to be readable live from the running PLC, so CONSTANT was the wrong storage class. read_running_version_online failed against EVERY project bumped via the old template -- 'Invalid expression' on the runtime read. Dropped CONSTANT from VERSION_GVL_DECLARATION_TEMPLATE in bump_project_version.py. Existing projects auto-migrate on the next bump because maintain_version_gvl()'s existing-GVL branch overwrites textual_declaration with the (now non-CONSTANT) template. The string is still effectively read-only at runtime -- only the bump tool updates it. read_running_version_online.py also got a more precise error message that explicitly fingerprints the 'Invalid expression' failure mode and points at the CONSTANT root cause. Useful for any user landing on a project that pre-dates this fix. Verified end-to-end against local CODESYS Control Win V3 (PLATEA, port 11740) on MCPTest2 v1.3.4.0: - connect_to_device, get_application_state, download_to_device, start_stop_application (both directions), read_variable (PLC_PRG.watchdog1 = 225 ticking), write_variable (200 -> 204 in 4s proves write took), disconnect_from_device: all 7 PASS. - read_running_version_online failure reproduced (CONSTANT inlined), fix landed -- next bump on MCPTest2 will validate. 37/37 unit/integration tests green. TEST_OVERVIEW.md updated with the v5 device sweep, with the headless-mode deep-dive, and with the broken-by-design notes on read_running_version_online.
This commit is contained in:
Karstein Phobic Nyvold Kvistad
parent
d414c779a5
commit
ef259c8ee3
7 changed files with 173 additions and 10 deletions
|
|
@ -27,9 +27,15 @@ VALID_LEVELS = ('major', 'minor', 'revision', 'build')
|
|||
# tool can pull it via online connect + read_variable. Kept as
|
||||
# qualified_only so it can't accidentally shadow a same-named local.
|
||||
VERSION_GVL_NAME = '_MCP_PROJECT_VERSION'
|
||||
# NOT VAR_GLOBAL CONSTANT: CODESYS inlines CONSTANT scalars at compile time,
|
||||
# which strips them from the online symbol table -- read_running_version_online
|
||||
# would then fail with 'Invalid expression' on every project bumped via this
|
||||
# tool. Plain VAR_GLOBAL keeps the symbol live so the online tool can read
|
||||
# the running version. The string is still effectively read-only at runtime
|
||||
# (only bump_project_version updates it via textual_declaration.replace).
|
||||
VERSION_GVL_DECLARATION_TEMPLATE = (
|
||||
"{attribute 'qualified_only'}\n"
|
||||
"VAR_GLOBAL CONSTANT\n"
|
||||
"VAR_GLOBAL\n"
|
||||
" sVersion : STRING := '%s';\n"
|
||||
"END_VAR\n"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -77,3 +77,125 @@ def ensure_online_connection(primary_project):
|
|||
|
||||
return online_app, target_app
|
||||
# --- End of ensure_online_connection function ---
|
||||
|
||||
|
||||
# --- Function to ensure the online application is logged in ---
|
||||
def ensure_logged_in(online_app, login_wait_seconds=30):
|
||||
"""Idempotently log into the device. In persistent mode the login state
|
||||
survives across calls and this is a no-op via online_app.is_logged_in.
|
||||
In headless mode each tool call spawns a fresh CODESYS process, so any
|
||||
online tool (start_stop, read_variable, write_variable,
|
||||
read_running_version_online) needs to log in itself before its action.
|
||||
|
||||
Without this helper, tools other than connect_to_device + download_to_device
|
||||
fail in headless mode with 'Application not logged in.' (start/stop) or
|
||||
'Invalid expression' (read/write).
|
||||
|
||||
Mirrors the SP-version-drift login probe in connect_to_device.py:
|
||||
OnlineChangeOption (older) vs LoginMode (SP21+) vs members on the
|
||||
online_app object itself; (mode, force_download_bool) two-arg shape vs
|
||||
(mode,) one-arg shape vs no-arg fallback. Prefers least-invasive
|
||||
'TryOnlineChange' / 'OnlineChangeOnly' semantics."""
|
||||
import scriptengine as _se # local import; ensure_online_connection.py
|
||||
# is concatenated into the script body so
|
||||
# the top-level import in the main script
|
||||
# is also visible, but keep this defensive.
|
||||
# Short-circuit: already logged in (persistent mode).
|
||||
if hasattr(online_app, 'is_logged_in'):
|
||||
try:
|
||||
if online_app.is_logged_in:
|
||||
print("DEBUG: ensure_logged_in: already logged in (persistent session).")
|
||||
return
|
||||
except Exception as e:
|
||||
print("DEBUG: ensure_logged_in: is_logged_in property raised: %s" % e)
|
||||
|
||||
if not hasattr(online_app, 'login'):
|
||||
raise TypeError("Online application does not support login().")
|
||||
|
||||
# Build enum candidate list -- same logic as connect_to_device.py's
|
||||
# main body, kept here so headless tool calls don't have to
|
||||
# re-implement it. Probe both script_engine.* and online_app.* for
|
||||
# LoginMode / OnlineChangeOption.
|
||||
enum_sources = []
|
||||
for src_name in ('LoginMode', 'OnlineChangeOption'):
|
||||
if hasattr(_se, src_name):
|
||||
try:
|
||||
enum_sources.append((src_name, getattr(_se, src_name)))
|
||||
except Exception:
|
||||
pass
|
||||
for src_name in ('LoginMode', 'OnlineChangeOption'):
|
||||
if hasattr(online_app, src_name):
|
||||
try:
|
||||
enum_sources.append(('online_app.' + src_name, getattr(online_app, src_name)))
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
preferred_order = ('TryOnlineChange', 'OnlineChangeOnly', 'Try', 'OnlineChange',
|
||||
'WithDownload', 'ForceDownload', 'Download',
|
||||
'None_', 'None')
|
||||
enum_candidates = []
|
||||
seen_keys = set()
|
||||
for src_name, oc in enum_sources:
|
||||
try:
|
||||
members = sorted([m for m in dir(oc) if not m.startswith('_')])
|
||||
except Exception:
|
||||
members = []
|
||||
for preferred in preferred_order:
|
||||
if preferred in members:
|
||||
key = '%s.%s' % (src_name, preferred)
|
||||
if key not in seen_keys:
|
||||
try:
|
||||
enum_candidates.append((key, getattr(oc, preferred)))
|
||||
seen_keys.add(key)
|
||||
except Exception:
|
||||
pass
|
||||
for m in members:
|
||||
key = '%s.%s' % (src_name, m)
|
||||
if key not in seen_keys:
|
||||
try:
|
||||
enum_candidates.append((key, getattr(oc, m)))
|
||||
seen_keys.add(key)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
call_shapes = []
|
||||
for nm, val in enum_candidates:
|
||||
call_shapes.append(("login(%s, False)" % nm, (val, False)))
|
||||
call_shapes.append(("login(%s, True)" % nm, (val, True)))
|
||||
call_shapes.append(("login(%s)" % nm, (val,)))
|
||||
call_shapes.append(("login(False)", (False,)))
|
||||
call_shapes.append(("login(True)", (True,)))
|
||||
call_shapes.append(("login()", ()))
|
||||
|
||||
last_err = None
|
||||
logged_in = False
|
||||
for desc, args in call_shapes:
|
||||
try:
|
||||
online_app.login(*args)
|
||||
print("DEBUG: ensure_logged_in: %s succeeded" % desc)
|
||||
logged_in = True
|
||||
break
|
||||
except Exception as e:
|
||||
last_err = e
|
||||
|
||||
if not logged_in:
|
||||
raise RuntimeError("ensure_logged_in: all login() call shapes failed. Last error: %s" % last_err)
|
||||
|
||||
# Settle wait. Same STABLE_STATES as connect_to_device.
|
||||
STABLE_STATES = ('run', 'stop', 'connected', 'halt', 'breakpoint')
|
||||
state = "unknown"
|
||||
for elapsed in range(login_wait_seconds):
|
||||
if hasattr(online_app, 'application_state'):
|
||||
try:
|
||||
state = str(online_app.application_state)
|
||||
except Exception:
|
||||
pass
|
||||
if state.lower() in STABLE_STATES:
|
||||
print("DEBUG: ensure_logged_in: state stabilised at '%s' after %ds" % (state, elapsed))
|
||||
return
|
||||
try:
|
||||
_se.system.delay(1000)
|
||||
except Exception:
|
||||
pass
|
||||
print("DEBUG: ensure_logged_in: state did not stabilise within %ds (last='%s'); proceeding anyway." % (login_wait_seconds, state))
|
||||
# --- End of ensure_logged_in function ---
|
||||
|
|
|
|||
|
|
@ -23,6 +23,8 @@ try:
|
|||
online_app, target_app = ensure_online_connection(primary_project)
|
||||
app_name = getattr(target_app, 'get_name', lambda: 'Unknown')()
|
||||
print("DEBUG: connected to application '%s'" % app_name)
|
||||
# Auto-login -- idempotent in persistent mode, required in headless.
|
||||
ensure_logged_in(online_app)
|
||||
|
||||
# Read the version anchor
|
||||
raw_value = None
|
||||
|
|
@ -36,7 +38,24 @@ try:
|
|||
raw_value = result
|
||||
except Exception as e:
|
||||
msg = str(e)
|
||||
if 'not found' in msg.lower() or 'unknown' in msg.lower() or 'symbol' in msg.lower():
|
||||
msg_l = msg.lower()
|
||||
if 'invalid expression' in msg_l:
|
||||
# Most common cause: the GVL is declared VAR_GLOBAL CONSTANT,
|
||||
# which CODESYS inlines at compile time -- the symbol never
|
||||
# makes it into the online table. This is a known footgun
|
||||
# because older bump_project_version emitted CONSTANT GVLs.
|
||||
raise RuntimeError(
|
||||
"Online evaluator returned 'Invalid expression' for '%s'. "
|
||||
"Most likely cause: the _MCP_PROJECT_VERSION GVL was created "
|
||||
"with VAR_GLOBAL CONSTANT, which CODESYS inlines at compile "
|
||||
"time so the symbol is not in the online symbol table. "
|
||||
"Fix: edit _MCP_PROJECT_VERSION's declaration to drop "
|
||||
"CONSTANT (just VAR_GLOBAL), then bump_project_version + "
|
||||
"download_to_device. (newer bump_project_version emits "
|
||||
"non-CONSTANT GVLs by default, so future-bumped projects "
|
||||
"are unaffected.) Underlying error: %s" % (VARIABLE_PATH, e)
|
||||
)
|
||||
if 'not found' in msg_l or 'unknown' in msg_l or 'symbol' in msg_l:
|
||||
raise RuntimeError(
|
||||
"Variable '%s' not found on the running PLC. "
|
||||
"Either bump_project_version has never been run on this project "
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@ try:
|
|||
|
||||
online_app, target_app = ensure_online_connection(primary_project)
|
||||
app_name = getattr(target_app, 'get_name', lambda: "Unknown")()
|
||||
# Auto-login -- idempotent in persistent mode, required in headless.
|
||||
ensure_logged_in(online_app)
|
||||
|
||||
# Read the variable value
|
||||
value = None
|
||||
|
|
|
|||
|
|
@ -13,6 +13,10 @@ try:
|
|||
|
||||
online_app, target_app = ensure_online_connection(primary_project)
|
||||
app_name = getattr(target_app, 'get_name', lambda: "Unknown")()
|
||||
# In headless mode each MCP call spawns a fresh CODESYS process, so the
|
||||
# login from a prior connect_to_device call is gone. ensure_logged_in
|
||||
# is idempotent in persistent mode (short-circuits via is_logged_in).
|
||||
ensure_logged_in(online_app)
|
||||
|
||||
if action_lower == 'start':
|
||||
if hasattr(online_app, 'start'):
|
||||
|
|
|
|||
|
|
@ -12,6 +12,8 @@ try:
|
|||
|
||||
online_app, target_app = ensure_online_connection(primary_project)
|
||||
app_name = getattr(target_app, 'get_name', lambda: "Unknown")()
|
||||
# Auto-login -- idempotent in persistent mode, required in headless.
|
||||
ensure_logged_in(online_app)
|
||||
|
||||
# SP21+/SP22 uses a two-step prepare-then-write pattern:
|
||||
# 1) set_prepared_value(name, value) -- stage the value
|
||||
|
|
|
|||
|
|
@ -109,16 +109,24 @@ Status legend: **✅ working** • **⚠ degraded** (works but with known gotcha
|
|||
|
||||
These all require a running PLC and a configured device gateway. Persistent timing here is **gateway-bound**, not CODESYS-bound — it's network roundtrips, not script overhead.
|
||||
|
||||
**v5 sweep verified end-to-end against local CODESYS Control Win V3** (PLATEA hostname, gateway port 11740) on 2026-04-26. All 8 tools called in sequence against MCPTest2 v1.3.4.0. **7/8 PASS, 1 broken-by-design (root-caused + fixed in this sweep, see notes).** Use **persistent mode** for any device-tool chain — headless spawns a fresh CODESYS process per call which kills the login state established by `connect_to_device`. Auto-login was added to all four scripts that previously relied on persisted login (start_stop_application, read_variable, write_variable, read_running_version_online) so they now also work end-to-end in headless. See "Headless mode + device tools" deep-dive below.
|
||||
|
||||
| Tool | Status | What it does | Persistent (with PLC) | Headless |
|
||||
|---|---|---|---|---|
|
||||
| `connect_to_device` | ✅ (fixed [`2607063`](https://github.com/phobicdotno/Codesys-MCP/commit/2607063), needs PLC to verify) | Logs into the active application via `online_app.login(...)`. Now probes 4 enum source locations (`script_engine.LoginMode`, `script_engine.OnlineChangeOption`, `online_app.LoginMode`, `online_app.OnlineChangeOption`) plus 3-arg call shape variant. | 1–5 s | n/a |
|
||||
| `disconnect_from_device` | ✅ (when connected) | `online_app.logout()` | 200–500 ms | n/a (no persistent online context) |
|
||||
| `get_application_state` | ✅ | Reads `online_app.application_state` (run/stop/halt/connected/...) | 100–300 ms (when online); 100 ms when offline | 8–14 s |
|
||||
| `read_variable` | ✅ (when connected) | `online_app.read_value('var.path')` over the gateway | 100–500 ms per call | n/a |
|
||||
| `write_variable` | ✅ (when connected) | `online_app.write_value('var.path', value)` | 100–500 ms | n/a |
|
||||
| `download_to_device` | ✅ (when connected) | Pushes the new boot application after a code change. Heavy. | 5–60 s (project size dependent) | n/a |
|
||||
| `start_stop_application` | ✅ (when connected) | `online_app.start()` / `.stop()` | 200–500 ms | n/a |
|
||||
| `read_running_version_online` | ✅ (when connected) | Reads `_MCP_PROJECT_VERSION.sVersion` from the running PLC | 100–500 ms | n/a |
|
||||
| `connect_to_device` | ✅ verified end-to-end (v5 sweep, fixed [`2607063`](https://github.com/phobicdotno/Codesys-MCP/commit/2607063)) | Logs into the active application via `online_app.login(...)`. Probes 4 enum source locations (`script_engine.LoginMode`, `script_engine.OnlineChangeOption`, `online_app.LoginMode`, `online_app.OnlineChangeOption`) plus 3-arg call shape variant. | 1–5 s | runs but login state lost on next call (use auto-login helper) |
|
||||
| `disconnect_from_device` | ✅ verified end-to-end (v5: `Logged In: True` → `False` confirmed) | `online_app.logout()` | 200–500 ms | n/a (no persistent online context) |
|
||||
| `get_application_state` | ✅ verified end-to-end (v5: returns `run`/`stop` correctly) | Reads `online_app.application_state` and `is_logged_in`. | 100–300 ms (when online); 100 ms when offline | 8–14 s (returns `none, Logged In: False` if not connected) |
|
||||
| `read_variable` | ✅ verified end-to-end (v5: `PLC_PRG.watchdog1 = BYTE#225` live, ticking) | `online_app.read_value('var.path')` over the gateway. **CONSTANT VAR_GLOBAL scalars are inlined at compile time and absent from the online symbol table -- expect 'Invalid expression'.** | 100–500 ms per call | now auto-logs-in (v5 fix) |
|
||||
| `write_variable` | ✅ verified end-to-end (v5: wrote 200 → read 204 4s later, 1 Hz tick proves write took) | `set_prepared_value` + `write_prepared_values` (SP21+ path), falls back to `write_value` / `set_value` / `write` / `set` for older SPs | 100–500 ms | now auto-logs-in (v5 fix) |
|
||||
| `download_to_device` | ✅ verified end-to-end (v5: pushed v1.3.4.0 to PLATEA) | Pushes the new boot application after a code change. Heavy. Has its own login probe (independent from auto-login helper). | 5–60 s (project size dependent) | runs end-to-end |
|
||||
| `start_stop_application` | ✅ verified end-to-end (v5: stop → `stop` state, start → `run` state) | `online_app.start()` / `.stop()` | 200–500 ms | now auto-logs-in (v5 fix) |
|
||||
| `read_running_version_online` | ⚠ **broken-by-design pre-v5; fixed in this sweep** | Reads `_MCP_PROJECT_VERSION.sVersion` from the running PLC. **Old `bump_project_version` emitted the GVL as `VAR_GLOBAL CONSTANT`, which CODESYS inlines at compile time, so the symbol never reaches the online evaluator -- read returns `Invalid expression`.** Fixed in v5 by dropping `CONSTANT` from `VERSION_GVL_DECLARATION_TEMPLATE`. Existing projects auto-migrate on next bump (the existing-GVL branch overwrites `textual_declaration` with the new template). Script also detects the failure mode and emits a precise actionable error. | 100–500 ms | now auto-logs-in (v5 fix) |
|
||||
|
||||
#### Headless mode + device tools (v5 deep-dive)
|
||||
|
||||
In **persistent** mode the MCP keeps one CODESYS process alive; `connect_to_device`'s login state survives across calls. In **headless** mode each MCP call spawns a fresh `CODESYS.exe --noUI` process — login state from the prior call is gone before the next call starts. Pre-v5, only `connect_to_device` and `download_to_device` did their own `online_app.login(...)`; the other four (`start_stop_application`, `read_variable`, `write_variable`, `read_running_version_online`) silently failed in headless with `Application not logged in.` (start/stop) or `Invalid expression` (read/write).
|
||||
|
||||
**v5 fix:** `ensure_logged_in(online_app, login_wait_seconds=30)` was added to `ensure_online_connection.py` next to the existing `ensure_online_connection`. The four affected scripts now call it immediately after creating the online app. The helper short-circuits via `online_app.is_logged_in` so persistent mode is a no-op (no extra login roundtrip), then runs the same enum-probe + call-shape probe + STABLE_STATES settle-wait pattern that `connect_to_device` and `download_to_device` already use. Net effect: every online tool now works end-to-end in BOTH modes.
|
||||
|
||||
### Git wrappers (6)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue