feat(network): add Wake-on-LAN wake module (type: wol) - #217
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Wake-on-LAN (WoL) “power switch” implementation and wires it into the existing powerControlClass dispatch so RAFT can wake devices that are asleep/soft-off via a WoL magic packet.
Changes:
- Added
powerWolmodule that constructs and sends a WoL magic packet to a configurable broadcast address/UDP port. - Added
type == "wol"dispatch branch inpowerControlClassto instantiate the new module.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| framework/core/powerModules/wol.py | Implements WoL magic-packet creation and UDP broadcast send; includes wake-only semantics for powerOff()/reboot(). |
| framework/core/powerControl.py | Adds import and type: "wol" dispatch branch to construct powerWol. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c5d9760 to
0a32b41
Compare
powerControlClass had no way to wake a device that is asleep or in a Wake-on-LAN soft-off state. Add framework/core/powerModules/wol.py (powerWol) and a `type: "wol"` dispatch branch: - powerOn() sends the WoL magic packet (6x 0xFF + MAC x16) to broadcast:port. - Wake-on-LAN is wake-only: powerOff() is a logged no-op, reboot() a best-effort wake. - Config: mac (required), broadcast (default 255.255.255.255), port (default 9). - Standard-library only (socket); no new dependencies. Fixes rdkcentral#216
…ake seam
Wake-on-LAN is a network (layer-2) action, not a power action: it sends a
UDP magic packet, cannot power a device off, and supports none of the power
metering contract. Move it out of powerModules onto a dedicated network
controller that mirrors the existing device-controller pattern
(remoteController / hdmiCECController / avSyncController).
- Add framework/core/networkModules/wol.py (networkWol) with a single wake()
verb (renamed from powerOn; drop the power-only powerOff/reboot shims).
- Add framework/core/networkControl.py (networkControlClass) dispatching
type: "wol", with retry, mirroring powerControl.
- Wire a device `network:` config block in deviceManager -> dut.networkController;
expose it in testControl.
- Revert the power-seam coupling: remove the wol import + type:"wol" branch
from powerControl.py and delete powerModules/wol.py.
- Document the `network:` block in the example rack config.
- Add tests/networkControl_tests.py (packet correctness, loopback capture,
MAC validation, unknown-type handling).
Config now:
network:
type: "wol"
mac: "aa:bb:cc:dd:ee:ff"
broadcast: "255.255.255.255" # optional
port: 9 # optional
Waking is now an explicit dut.networkController.wake() rather than riding on
powerControl.powerOn().
Relates to rdkcentral#216
64dff49 to
1a27ddd
Compare
|
Updated and ready for review. This revision re-homes Wake-on-LAN from the power seam onto a dedicated network/wake controller ( dut.networkController.wake()@TB-1993 could you take another look and approve when happy? Thanks 🙏 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
framework/core/networkControl.py:59
- If
config['type']is unknown, this constructor only logs an error and leavesself.networkModuleasNone. A subsequentwake()call will then raiseAttributeErrorwhen it tries to accessself.networkModule.wake. Also, assigning to a local namedtypeshadows the Python built-in.
self.networkModule = None
type = config.get("type")
if type == "wol":
self.networkModule = networkWol(log, config.get("mac"), config.get("broadcast", "255.255.255.255"), config.get("port", 9))
else:
self.log.error("Network controller type [{}] unknown".format(type))
| # If variables are not passed in the config they will be defaulted to retryCount: [1], retryDelay: [30] | ||
| self.retryCount = config.get("retryCount", 1) | ||
| self.retryDelay = config.get("retryDelay", 30) |
There was a problem hiding this comment.
Should retries really come from the config. Not the test?
There was a problem hiding this comment.
thank you read that wrong. This is coming from the config, config.get("retryCount", 1) it just has a default if the config isn't present.
the retry count is a system control not a test control, that's what I required, that would then mean that the hardware we're talking too has a specification, and if it doesn't meet that then it fails... And that's why it's setup this way.
It's not ok for a test to think it knows the specification of the hardware.
There was a problem hiding this comment.
In this case the hardware is the DUT, right? If that's the case I would expect the retry count to be in the device_config.yml and the test to read it and pass it in at run time. I could be missing something though?
…iew) Addresses review feedback on the WoL network module: - Validate/normalise the MAC at construction (strip whitespace, require 12 hex digits) with a clear, field-named ValueError, instead of only checking length later and surfacing a generic bytes.fromhex() error. Misconfiguration now fails when the device is built, not silently at wake() time. - Rename _magicPacket -> _magic_packet for consistency with the other modules' snake_case private helpers; update the call site. - Guard networkControlClass.wake() when no module was constructed (unknown 'type') so it returns a clear error instead of an AttributeError on None.
|
Addressed the mechanical review points in 1fc2e62:
Still open for a design decision (from @TB-1993's comments) before I change them:
I'll implement these once the direction is agreed. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
framework/core/networkControl.py:95
networkRetry()currently stops after the first call regardless of the returned value (it only retries on exceptions). SincenetworkWol.wake()returnsFalseon send errors (it does not raise),retryCount/retryDelaywill never be applied for WoL failures. UpdatenetworkRetry()to treat a falsy return as a failure and keep retrying until attempts are exhausted.
for x in range(self.retryCount+1):
try:
result = networkMethod()
break
except Exception as e:
self.log.info(f"Could not perform {[networkMethod.__name__]}. Retry count: [{x}] of [{self.retryCount}], delay is [{self.retryDelay}]")
if x == self.retryCount:
raise e
self.utils.wait(self.retryDelay)
return result
…anch The Wake-on-LAN work was re-homed upstream from a power seam to a network/wake module (rdkcentral/python_raft#217); pinning the moving branch name meant a rebase/force-push on the PR could silently change what install.sh resolves. Pin the immutable #217 head commit (1fc2e62) instead, and note the network/wake shape. Revert to the upstream tag once #217 merges and lands in a release (post-1.9.0).
…st (review) - networkRetry now retries on a falsy return as well as on exceptions, up to retryCount with a fixed retryDelay (no exponential backoff -- a WoL packet either lands or it does not). Previously it only retried on exceptions, so the configured retries never fired for a failed send. Also fixes the retry log message (bare method name instead of "['wake']") and no longer re-raises. - tests/networkControl_tests.py is now a unittest.TestCase (assertions, CI-runnable) instead of a manual print script: magic-packet correctness, MAC normalisation, fail-fast invalid-MAC, wake() send over loopback, unknown-type guard, and retry-count behaviour.
|
Follow-up on the design threads (39d0dd0):
That should clear the outstanding threads bar the two intentional follow-ups. Let me know if the retry direction or anything else needs a different call. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
framework/core/networkControl.py:48
self.namemay beNonewhen nonamefield is provided in the devicenetwork:block, which leads to confusing log messages likewake (None)/wake (None): no network module configured. Consider defaulting it similarly topowerControlClassso logs are informative out of the box (e.g. usemacortype).
self.name = config.get("name")
…message) Addresses remaining Copilot log-clarity comments: default the controller name to the type so logs read 'wake (wol)' not 'wake (None)', and state the actual destination (broadcast:port for the target MAC) in the wake log line.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
framework/core/networkControl.py:60
- When the
network:block is present but missingtype, the log currently saysNetwork controller type [None] unknown, which is ambiguous (missing vs unsupported). Handling theNonecase explicitly also letsself.nameand subsequentwake(...)logs be clearer.
type = config.get("type")
if type == "wol":
self.networkModule = networkWol(log, config.get("mac"), config.get("broadcast", "255.255.255.255"), config.get("port", 9))
else:
self.log.error("Network controller type [{}] unknown".format(type))
| self.broadcast = broadcast or "255.255.255.255" | ||
| self.port = int(port) if port else 9 | ||
| self.log.info("networkWol(mac={}, broadcast={}, port={})".format( | ||
| self.mac, self.broadcast, self.port)) |
Closes #216.
What
Adds a network/wake controller for a device — Wake-on-LAN lives here, not in the power seam.
framework/core/networkModules/wol.py(networkWol) — a singlewake()verb that sends the WoL magic packet (6×0xFF+ target MAC ×16) tobroadcast:port.framework/core/networkControl.py(networkControlClass) — dispatchestype: "wol", with the same retry behaviour (retryCount/retryDelay) aspowerControl.deviceManagervia a devicenetwork:block → exposed asdut.networkController; also surfaced ontestControlasself.networkController.socket) — no new dependencies.Usage in a suite is deliberately dead-simple:
Why network, not power
Wake-on-LAN is a network (layer-2) action, not a power action: it sends a UDP magic packet, cannot power a device off, and supports none of the power-metering contract (
getPowerLevel/getVoltageLevel/getCurrentLevel). Filing it underpowerModulesmislabelled it and left half the "power switch" contract inapplicable.So it is re-homed onto a dedicated network controller that mirrors the existing device-controller pattern (
remoteController/hdmiCECController/avSyncController). The problem it solves is unchanged: targets that aggressively deep-sleep drop their console when idle and can only be brought back by a magic packet; without this, runs against them need an out-of-band wake step before the session opens.Behavioural note: waking is now an explicit
dut.networkController.wake()rather than riding onpowerControl.powerOn(). The seam is designed to grow (e.g.isReachable()/waitForOnline()as a natural companion towake();deviceClass.pingTest()already offers ICMP reachability).Testing
tests/networkControl_tests.py— buildsnetworkControlClassfrom config and callswake().6×0xFF+ MAC×16),wake()sends and was captured on loopback, missing / malformed MAC raiseValueError, unknown type is handled gracefully.network:block producesdut.networkController;powerControl(type:"wol")now correctly reports the type as unknown.Docs
AGENTS.md— directory tree, controller shortcut attributes, and a new §5.2 Network / wake modules (broadened §5 to "Power & Network Control").docs/modules/device_manager.md—networkControlleradded to thedeviceClassmanaged objects.Note: the branch name (
…power-module) predates the re-home; the code is a network module. First commit adds the initial (power-seam) cut; the second re-homes it to the network seam and reverts the power coupling.