Skip to content

power: return the unsure state from the shellcmd driver - #609

Merged
chombourger merged 1 commit into
siemens:nextfrom
Poseidonas:shellcmd-status-return-unsure
Aug 24, 2026
Merged

power: return the unsure state from the shellcmd driver#609
chombourger merged 1 commit into
siemens:nextfrom
Poseidonas:shellcmd-status-return-unsure

Conversation

@Poseidonas

Copy link
Copy Markdown

docs/config.rst describes check-on as:

Should return 0 if power is on, 1 if it is off. Any other return code is interpreted as error.

The error branch in ShellCmdPowerController.status() evaluates self.POWER_UNSURE without returning it, so the method falls through and yields None.

That None reaches the user rather than staying internal. grpc/servicer.py:478 sends str(result or ''), so mtda-cli target status prints an empty line where the documentation says it should report an unknown state.

Driving the controller with subprocess.run patched, before and after:

check-on exit=  0   ON     -> 'ON'
check-on exit=  1   OFF    -> 'OFF'
check-on exit=  2   None   -> ''      after: '???'
check-on exit=127   None   -> ''      after: '???'

The change is the missing return.

I scanned the rest of the tree with the AST for the same shape — a bare self.X expression statement where a return was meant — and this is the only occurrence.

flake8 is clean with the exclusions from tox.ini. I could not run scripts/test-using-docker here (the archlinux image has no linux/arm64 manifest, and mtda-service imports systemd, which is not installable on macOS), so this rests on the isolated check above.

Found while looking at #596, which turns out to be already fixed by 9b2890e — the documentation now reads correctly, but the code no longer matches it in this one branch. Worth closing #596 if it was left open by oversight.

The documented contract for check-on is "Should return 0 if power is on, 1
if it is off. Any other return code is interpreted as error". The error
branch evaluates self.POWER_UNSURE without returning it, so status() yields
None instead.

That None reaches the user: the gRPC servicer sends str(result or ''), so
`mtda-cli target status` prints an empty line where it should print ??? .

    check-on exit=0    ON      ON
    check-on exit=1    OFF     OFF
    check-on exit=2    None    ''      <- should be ???
    check-on exit=127  None    ''      <- should be ???

Verified by driving the controller with subprocess.run patched.

Signed-off-by: George Vasiliades <1221374+Poseidonas@users.noreply.github.com>
@chombourger

Copy link
Copy Markdown
Collaborator

good catch and thanks for being so thorough (e.g. use of AST to check for similar issues in the code tree)

@chombourger
chombourger changed the base branch from main to next August 24, 2026 13:44
@chombourger
chombourger merged commit 377f4ac into siemens:next Aug 24, 2026
5 checks passed
@Poseidonas Poseidonas moved this from Waiting on review to Done in Industrial open source Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants