[ddwrt] Initial contribution of DD-WRT/OpenWrt/Tomato SSH based binding - #20505
Conversation
|
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/considering-a-new-dd-wrt-binding/162386/9 |
lsiepel
left a comment
There was a problem hiding this comment.
Thanks, this is a first pass, review half of the files to get an impression and i think these comments can be applied on multiple places. Once these are addressed we can continue with the next review iteration.
From AI review of PR openhab#20505 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
AI code review of PR openhab#20505 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
AI code review PR openhab#20505 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
review in PR openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
code review PR openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
code review in PR openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
PR code review openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
code review PR openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
…an up discovery logging - Add semantic equipment tags to all thing types (Router, NetworkAppliance, WirelessAccessPoint, Firewall) - Add semantic tags to channels (Status, Measurement, Control, Switch, Timestamp, Temperature, Level, RSSI, SignalStrength, Speed) - Add keywords to addon.xml for better discoverability - Rename assoclist channel to assoc-list for consistency with other channel IDs - Simplify discovery service: only include hostname and mac in properties, remove redundant debug logging PR openhab#20505 Assisted-by: Windsurf:SWE-1.6 Signed-off-by: Lee Ballard <ballle98@gmail.com>
|
I re-opened some comments. When i try to build the binding locally, many tests fail. I also see build errors, have not checked SAT, please check that too. |
Looks like the CI build passed. And I ran a CI build locally with no issues. There are no issues in target/summary_report.html |
From AI review of PR openhab#20505 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
AI code review of PR openhab#20505 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
AI code review PR openhab#20505 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
review in PR openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
code review PR openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
code review in PR openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
PR code review openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
- Change 8 non-compliant logger.info calls to logger.debug per openHAB guidelines (info should only be used for newly started components or loaded files) - Remove MAC address from wireless client discovery properties PR openhab#20505 Assisted-by: Windsurf:SWE-1.6 Signed-off-by: Lee Ballard <ballle98@gmail.com>
PR openhab#20505 Assisted-by: Windsurf:SWE-1.6 Signed-off-by: Lee Ballard <ballle98@gmail.com>
- SyslogParser: Reuse static Gson instance instead of per-call allocation - SyslogParser: Add Locale.ENGLISH to timestamp DateTimeFormatter - SshRunner: Add timeout to ch.open().verify() to prevent indefinite blocking - SshClientManager: Add shutdown() method for OSGi lifecycle cleanup - DDWRTHandlerFactory: Call SshClientManager.shutdown() on deactivate - DDWRTRadioThingHandler: Pass ifaceName (not interfaceId) to setRadioEnabled - DDWRTRadioThingHandler: Normalize interfaceId in initialize for manual config - thing-types.xml: Update radio interfaceId description to document mac:iface format - ddwrt.properties: Update radio interfaceId description - DDWRTBaseDevice: Change radioName separator from hyphen to space PR openhab#20505 Assisted-by: Windsurf:Claude Opus 4.6 Signed-off-by: Lee Ballard <ballle98@gmail.com>
PR openhab#20505 Assisted-by: Windsurf:Claude Opus 4.6 Signed-off-by: Lee Ballard <ballle98@gmail.com>
1. DDWRTDiscoveryService.java — Null-guard on inbox (line 246) 2. DDWRTDiscoveryService.java — Wrong thing type filter (line 257) 3. DDWRTClientHandler.java — Missing status on config error (line 78) PR openhab#20505 Assisted-by: Windsurf:Claude Opus 4.6 Signed-off-by: Lee Ballard <ballle98@gmail.com>
1. DDWRTDiscoveryService.java (line 154) moved properties from discovery to refresh. 2. addon.xml (line 9) removed duplicate addon keywords PR openhab#20505 Assisted-by: Windsurf:Claude Opus 4.6 Signed-off-by: Lee Ballard <ballle98@gmail.com>
Fixes #68 Assisted-by: Windsurf:SWE-1.6 Fast Assisted-by: Copilot:GPT 5.5 Think Deeper Signed-off-by: Lee Ballard <ballle98@gmail.com>
|
@lsiepel please review. Install from marketplace should work now. |
|
@lsiepel I believe I have addressed all your comments. Did I miss anything? Do you have additional comments? |
lsiepel
left a comment
There was a problem hiding this comment.
Another pass, not yet fully covered everything, but don;t want to hold back these comments.
| <category>QualityOfService</category> | ||
| <tags> | ||
| <tag>Measurement</tag> | ||
| <tag>SignalStrength</tag> |
There was a problem hiding this comment.
Not sure about the signalstrength, as it is more quality of sigtnal
PR openhab#20505 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
PR openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
PR openhab#20505 Assisted-by: Devin:Claude Opus 4.6 Signed-off-by: Lee Ballard <ballle98@gmail.com>
lsiepel
left a comment
There was a problem hiding this comment.
Thanks, i have now covered all files. Found a few more in the handlers.
PR openhab#20505 Co-authored-by: lsiepel <leosiepel@gmail.com> Signed-off-by: Lee Ballard <ballle98@gmail.com>
- Replace generic catch(Exception) with catch(IOException | RuntimeException) in DDWRTBaseDevice, DDWRTNetwork, SshAuthSession, and DDWRTDiscoveryService - Remove redundant try-catch around SshAuthSession.close() in DDWRTBaseDevice.closeSessionQuietly() - Add debug logging for NumberFormatException in DDWRTBroadcomDevice (noise, channel, temperature parsing) - Remove section splitter comments in DDWRTClient - Downgrade logger.error to logger.warn for SSH command failures in SshRunner - Refactor SyslogParser to accept ZoneId at construction and derive currentYear internally, removing the year parameter from parseLine() - Extract property key constants (INTERFACE_ID, PARENT_DEVICE_MAC, MAC, RULE_ID, CHIPSET) into DDWRTBindingConstants - Extract REFRESH_DEBOUNCE_SECONDS constant in DDWRTDiscoveryService - Validate hostnames config in DDWRTNetworkBridgeHandler.initialize() with i18n error message - Remove MAC from client discovery properties (unstable with randomization) - Update README to reflect channel moves from device to network bridge, new client/device channels, and updated thing configuration - Update SyslogParserTest for revised parseLine() signature PR openhab#20505 Assisted-by: Devin:Claude Opus 4.6 Signed-off-by: Lee Ballard <ballle98@gmail.com>
lsiepel
left a comment
There was a problem hiding this comment.
Thanks, while there are still some minor cleanups possible, i think the model is solid and as this is a new binding the risk is about zero and certainly not warrants postponing for another 6 months.
LGTM
You could add the addon's logo to the openHAB website. See https://www.openhab.org/docs/developer/addons/#add-your-add-on-s-logo-to-the-openhab-website-and-the-ui
New binding that monitors and manages DD-WRT, OpenWrt, Tomato, and generic Linux routers/access points via SSH. It auto-detects the wireless chipset driver (Broadcom wl, Atheros wl_atheros, Marvell iwinfo, OpenWrt iwinfo, generic iw) and adapts its commands accordingly.
Features:
Device telemetry — CPU load, CPU temperature, uptime, WAN IP, interface traffic counters
Wireless radio monitoring — SSID, channel, mode, associated client list
Wireless client tracking — online/offline state, signal strength, SNR, roaming between APs, MAC randomization support (hostname-based identification)
Syslog monitoring — real-time DHCP, wireless association, warning, and error events via logread -f, tail -F, or journalctl -f
Firewall rule control — enable/disable DD-WRT GUI-configured filter rules via nvram
Device reboot — remote reboot via SSH
Automatic discovery — devices on the network bridge, radios per device, wireless clients, and firewall rules
Thing types: network (bridge), device, radio, wireless-client, firewall-rule Channel types: 40 channels across 4 thing types covering telemetry, radio status, client details, syslog events (state + trigger), and firewall state
Architecture:
Single network bridge manages multiple devices; all things are children of the bridge
Per-device SSH session with automatic recovery on connection loss
Per-device refresh thread with catch-all exception handling for resilience
Chipset auto-detection from SSH MOTD banner and runtime probing
SshLogFollower with auto-reconnect for continuous syslog streaming
DDWRTNetworkCache (ConcurrentHashMap-based) for thread-safe device/client/radio state
OUI database for generating human-readable hostnames for devices without DHCP names
Ed25519 SSH key support in OSGi via explicit JCA provider registration
SSH ~/.ssh/config integration (ProxyJump, User, Port, IdentityFile)
Tests: 58 unit tests across 3 test classes (SyslogParserTest, OuiDatabaseTest, DDWRTNetworkCacheTest)
Dependencies:
Apache SSHD 2.17.1 (sshd-osgi), net.i2p.crypto eddsa 0.3.0
Prior discussion:
fix #18282
Community forum: https://community.openhab.org/t/considering-a-new-dd-wrt-binding/162386
Marketplace
https://community.openhab.org/t/dd-wrt-binding-5-1-0-0-6-0-0-0/168973
Tested with:
DD-WRT (Netgear R7000 Broadcom & Linksys WRT1900ACS Marvell chipsets) — gateway router
OpenWrt 24.10 (MediaTek MT7981, Cudy WR3000) — mesh AP