feat: centralize hayward command building#5
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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 static String buildSetHeaterEnable(String prefix, String token, int mspSystemID, String poolID, | ||
| String heaterID, String enabled) { | ||
| return prefix | ||
| + String.format(SET_HEATER_ENABLE, token, mspSystemID, poolID, heaterID, enabled) | ||
| + closingTag(prefix); | ||
| } | ||
|
|
||
| public static String buildSetUIHeaterCmd(String prefix, String token, int mspSystemID, String poolID, | ||
| String heaterID, String temp) { | ||
| return prefix + String.format(SET_UI_HEATER_CMD, token, mspSystemID, poolID, heaterID, temp) |
There was a problem hiding this comment.
[P0] Align mspSystemID parameter type with callers
The new CommandBuilder expects int mspSystemID in both buildSetHeaterEnable and buildSetUIHeaterCmd, but HaywardAccount.mspSystemID is defined as a String and the handlers pass that string directly. This mismatch means the binding no longer compiles because there is no implicit conversion from String to int. Either accept a String here or parse it at the call site before invoking the builder.
Useful? React with 👍 / 👎.
| public static String buildSetHeaterEnable(String prefix, String token, int mspSystemID, String poolID, | ||
| String heaterID, String enabled) { | ||
| return prefix | ||
| + String.format(SET_HEATER_ENABLE, token, mspSystemID, poolID, heaterID, enabled) | ||
| + closingTag(prefix); | ||
| } | ||
|
|
||
| public static String buildSetUIHeaterCmd(String prefix, String token, int mspSystemID, String poolID, | ||
| String heaterID, String temp) { | ||
| return prefix + String.format(SET_UI_HEATER_CMD, token, mspSystemID, poolID, heaterID, temp) |
There was a problem hiding this comment.
[P0] Use String for mspSystemID in local command builder
The local variant of CommandBuilder repeats the same int mspSystemID signature even though HaywardAccount.mspSystemID is a String. Calls from HaywardVirtualHeaterHandler now fail to compile because a String cannot be passed to these methods. The parameter should be a string (or callers should parse it to an integer) so the module builds.
Useful? React with 👍 / 👎.
matchews
deleted the
codex/create-commandbuilder-and-refactor-handlers
branch
1 participant
Summary
Testing
mvn -q -pl bundles/org.openhab.binding.haywardomnilogiclocal,bundles/org.openhab.binding.haywardomnilogic -am test(fails: Non-resolvable parent POM org.openhab:openhab-super-pom)https://chatgpt.com/codex/tasks/task_e_68c04a5197dc832380c432d974f8faa4