Refactor Hayward OmniLogic local binding to UDP#3
Conversation
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| public void scheduledInitialize() throws UnknownHostException { | ||
| config = getConfigAs(HaywardConfig.class); | ||
| udpClient = new UdpClient(config.endpointUrl, UDP_PORT); |
There was a problem hiding this comment.
[P1] Parse endpoint host before constructing UdpClient
The bridge now instantiates UdpClient with config.endpointUrl, but the configuration parameter is still documented as a full HTTP URL (see OH-INF/thing/bridge.xml, default https://www.haywardomnilogic.com/...). Passing a URL string to InetAddress.getByName results in an UnknownHostException before any login attempt, so existing installations will fail to initialise even with the default configuration. Extract the host portion (e.g., via URI) or adjust the configuration format before creating the client.
Useful? React with 👍 / 👎.
| public String send(UdpRequest request) throws IOException { | ||
| byte[] out = request.toBytes(); | ||
| try (DatagramSocket socket = new DatagramSocket()) { | ||
| DatagramPacket packet = new DatagramPacket(out, out.length, address, port); | ||
| socket.send(packet); | ||
|
|
||
| byte[] buf = new byte[4096]; | ||
| DatagramPacket responsePacket = new DatagramPacket(buf, buf.length); | ||
| socket.receive(responsePacket); |
There was a problem hiding this comment.
[P1] Add receive timeout to UdpClient
UdpClient.send creates a DatagramSocket and blocks on socket.receive(responsePacket) without setting SoTimeout. When the controller is offline or drops packets, the call hangs indefinitely and any init or polling job using this method never returns, whereas the previous HTTP client had a 10 s timeout and retry logic. Add a socket timeout and handle the resulting SocketTimeoutException so the thing can recover and update its status.
Useful? React with 👍 / 👎.
Summary
Testing
mvn -pl bundles/org.openhab.binding.haywardomnilogiclocal -am test(fails: Non-resolvable parent POM; network is unreachable)https://chatgpt.com/codex/tasks/task_e_68c0437bb21c83239bd1ff3d2aaa4000