Merge branch 'pr-1152' into master

Bring PR #1152 changes into master and resolve Messages.ts conflict by preserving both heater protocol response matching and non-IntelliCenter action fallback logic.

Made-with: Cursor

Author: sfc-gh-rgoldin

agent_briefs.md

@@ -0,0 +1,162 @@
+# Agent Briefs (3 Workstreams)
+
+## Goal
+
+Run 3 agents with minimal overlap:
+
+- Agent A: General tab data-path issues (Personal Info + Time/Locality + Delays)
+- Agent B: Alerts/Security mapping
+- Agent C: Loading-stuck/reliability behavior
+
+This split keeps shared protocol/parser work in one place and avoids conflicting edits across multiple agents.
+
+## Shared Context for All Agents
+
+- Discussion thread: <https://github.com/tagyoureit/nodejs-poolController/discussions/1090#discussioncomment-15925386>
+- Newly extracted replay folders:
+  - `data/1090/v3.008_replay.163`
+  - `data/1090/v3.008_replay.164`
+  - `data/1090/v3.008_replay.170`
+  - `data/1090/v3.008_replay.171`
+  - `data/1090/v3.008_replay.173`
+- Historical comparison replays:
+  - `data/1090/v3.008_replay.157_config1`
+  - `data/1090/v3.008_replay.160_config2`
+
+---
+
+# Agent A Brief: General Tab (Personal Info + Time/Locality + Delays)
+
+## Scope
+
+- Personal Information mapping (inbound + outbound)
+- Timezone/Clock settings and unintended coupling to body temps/modes
+- Delays tab and new v3 fields (`Frz Cycle Time`, `Frz Override`, `Manual Operation Priority`)
+
+## Why this is grouped
+
+These items share the same config/state pathways and overlap in `Action 30` + `Action 168` handling. Keeping them together reduces parser conflicts.
+
+## Exact old comment excerpts to use
+
+> "Personal Information: The information in this section does not appear to link to the OCP (and maybe it isn't intended to). In particular, the City/State/Zip does not link between what is shown on the dashPanel and the WCP. Also, the Pool Alias, Owner and Zip fields on the dashPanel change when other changes are made, with the contents of the Zip field replacing the Pool Alias and then the Zip and Owner fields becoming blank. The Longitude field can't be edited in dashPanel and it does not track changes made with the WCP. The Latitude field can't be edited and is blank in dashPanel."
+
+> "Timezone & Locality: Changing the settings from \"Internet / 12 hour\" to \"Manual / 24 Hour\" causes the Pool Set Point temp to change to 0 and the Spa Set Point Temp to change to 100 and the Spa Heat Mode to change from UltraTemp Only to Solar Only . (Replay 160)."
+
+> "Delays: The settings in the Delays tab do not appear to link between dashPanel and the WCP. Also, dashPanel has a \"Manual Operation Priority\" checkbox that doesn't appear to match anything on the WCP. And, the WCP has \"Frz Cycle Time (min)\" and \"Frz Overide (min)\" fields that do not appear in dashPanel."
+
+## Exact new comment excerpts to use
+
+> "Personal Information All fields except Lat/Lon entered on dashPanel are retained in dashPanel. Lat is blank. Lat/Lon are display only in dashPanel. Zip code, City, State and Country entered in dashPanel are not passed to WCP (other fields don’t exist in WCP). Zip code entered in WCP appears as Pool Alias in dashPanel.City entered in WCP appears as Phone in dashPanelState, Country, Latitude and Longitude entered in WCP are not reflected in dashPanel"
+
+> "There continues to be an incorrect linkage between the time settings and the Pool/Spa temperature settings."
+
+> "Setting Clock Source to “Server” on dashPanel causes error (Replay 163): TypeError: Cannot read properties of undefined (reading 'val') at IntelliCenterSystemCommands.setTZ (/home/pi/nodejs-poolController/controller/boards/SystemBoard.ts:1152:98) ..."
+
+> "Setting Clock Mode (12/24 hour) in dashPanel does transfer to WCP. However, if Clock Source is “Manual” in dashPanel and WCP, changing Clock Mode in dashPanel also causes WCP Mode to switch to “Internet” (Replay 164)."
+
+> "8:04 Changed Frz Cycle Time from 15 to 20 ... 8:24 Frz Override changed from 30 to 90"
+
+## Replay priority
+
+1. `v3.008_replay.171` (personal info + delays + many config changes)
+2. `v3.008_replay.170` (timezone/locality coupling)
+3. `v3.008_replay.164` (manual/internet mode interaction)
+4. `v3.008_replay.163` (server clock source exception)
+5. Cross-check with `v3.008_replay.160_config2`
+
+## Deliverables
+
+- Byte-level mapping table (field -> action/type -> payload offsets -> direction)
+- Confirmed root-cause list per symptom
+- Patch list (file + method + behavior)
+- Regression checklist for General tab
+
+---
+
+# Agent B Brief: Alerts and Security
+
+## Scope
+
+- Populate/validate Alerts tab mappings
+- Populate/validate Security tab mappings
+- Verify round-trip behavior for WCP/OCP <-> njsPC <-> dashPanel
+
+## Exact old comment excerpts to use
+
+> "Alerts and Security: These tabs are currently empty. I will have to take a further look at the OCP/WCP menus to see if there are corresponding entries."
+
+## Exact new comment excerpts to use
+
+> "Alerts and Notifications-Circuits 8:07 Alert Valve Rotation Delay from Off to On 8:08 Heater Cool Down Delay from Off to On"
+
+> "Alerts and Notifications-Pumps NOTE: All of the Alerts in this section were changed from On to Off ... 8:09 Pentair VS/VF/VSF Rate and Power ... 8:11 Communication Lost"
+
+> "Alerts and Notifications-Ultra Temp Heater NOTE: All of the Alerts in this section were changed from On to Off ... 8:17 Communication Lost"
+
+> "Alerts and Notifications-IntelliChlor NOTE: All of the Alerts in this section were changed from On to Off ... 8:19 Communication Lost"
+
+> "Start of REPLAY 173 ... 9:30 Enable Security – Administrator ... 9:32 Change Security Passcode from 7777 to 8888 ... 9:32 Enable Guest ... 9:33 Add “Vacation Mode” access to Guest ... 9:33 Add “All” access to Guest"
+
+## Replay priority
+
+1. `v3.008_replay.171` (alerts changes)
+2. `v3.008_replay.173` (security changes)
+
+## Deliverables
+
+- Mapping matrix for each alert/security setting
+- Missing fields report (present on WCP/OCP but absent in dashPanel/njsPC)
+- Patch list and validation notes
+- Any required API schema updates for dashPanel consumption
+
+---
+
+# Agent C Brief: Loading-Stuck and Reliability
+
+## Scope
+
+- Investigate root causes of loading hang/stalls under noisy comms
+- Evaluate refresh-trigger behavior (ACK(168)/ACK(184)/piggyback/config queue churn)
+- Propose mitigation strategy that does not suppress valid updates
+
+## Exact old comment excerpts to use
+
+> "A general issue which may be specific to my system is that periodically following a settings change initiated in dashPanel, the \"Loading\" message will appear and will hang before completion. That occurred at the end of Replay 157 with the updated hanging at 0%. Although nodejs-poolController continues to run, I have to restart it to clear the problem. My system is showing a lot of failed sends (55% failure rate) which might be related to this."
+
+## Exact new comment excerpts to use
+
+> "I restarted the access point that the Elfin uses and have had significantly fewer transmit errors since then."
+
+> "[12/24/2025, 10:47:00 AM] warn: Registration rejected by OCP via Action 217 (status=4) - duplicate registration attempt?"
+
+## Replay priority
+
+1. `v3.008_replay.157_config1` (stuck at loading 0)
+2. `v3.008_replay.160_config2` (comparison run)
+3. Correlate with new runs:
+   - `v3.008_replay.163`
+   - `v3.008_replay.164`
+   - `v3.008_replay.170`
+   - `v3.008_replay.171`
+   - `v3.008_replay.173`
+
+## Deliverables
+
+- Event-timeline of one failed and one successful config cycle
+- Trigger/churn analysis (`Requesting IntelliCenter configuration`, queued items, completion rates)
+- Proposed mitigation options with tradeoffs:
+  - refresh debounce tuning
+  - in-flight refresh guard
+  - stale loading-state timeout/reset behavior
+  - registration retry/backoff adjustments
+- Recommended implementation order
+
+---
+
+# Coordination Rules (Important)
+
+- Agent A owns changes in shared General/config parser paths.
+- Agent B should not modify Agent A parser logic unless required and documented.
+- Agent C should avoid behavior changes that alter field parsing/mapping.
+- Final integration step should run all touched replay scenarios before merge.

controller/boards/EasyTouchBoard.ts

@@ -1218,6 +1218,26 @@ class TouchSystemCommands extends SystemCommands {
     }
 }
 class TouchBodyCommands extends BodyCommands {
+    public getHeatSources(bodyId: number) {
+        let heatSources = super.getHeatSources(bodyId);
+        for (let i = 0; i < heatSources.length; i++) {
+            let hm = heatSources[i];
+            if (hm?.name === 'ultratemp' && typeof hm.val === 'undefined') {
+                heatSources[i] = this.board.valueMaps.heatSources.transformByName('heatpump');
+            }
+        }
+        return heatSources;
+    }
+    public getHeatModes(bodyId: number) {
+        let heatModes = super.getHeatModes(bodyId);
+        for (let i = 0; i < heatModes.length; i++) {
+            let hm = heatModes[i];
+            if (hm?.name === 'ultratemp' && typeof hm.val === 'undefined') {
+                heatModes[i] = this.board.valueMaps.heatModes.transformByName('heatpump');
+            }
+        }
+        return heatModes;
+    }
     public async setBodyAsync(obj: any): Promise<Body> {
         // The 168 is a funky packet in *Touch because it can set:
         // * Intellichem Installed (byte 3, bit 1)
@@ -2702,6 +2722,18 @@ class TouchHeaterCommands extends HeaterCommands {
         let heaters = sys.heaters.get();
         let types = sys.board.valueMaps.heaterTypes.toArray();
         let inst = { total: 0 };
+        // If NCP directly controls an ultratemp/heatpump for the same body, suppress
+        // corresponding OCP ghost entries for that body only.
+        let hasNcpUltratempForBody = (ocpHeater: Heater): boolean => {
+            let ocpBody = typeof ocpHeater.body === 'number' ? ocpHeater.body : 32;
+            return heaters.some(h => {
+                if (h.master !== 1 || h.isActive === false) return false;
+                let t = types.find(elem => elem.val === h.type);
+                if (!t || (t.name !== 'ultratemp' && t.name !== 'heatpump')) return false;
+                let ncpBody = typeof h.body === 'number' ? h.body : 32;
+                return ncpBody === 32 || ocpBody === 32 || ncpBody === ocpBody;
+            });
+        };
         for (let i = 0; i < types.length; i++) if (types[i].name !== 'none') inst[types[i].name] = 0;
         for (let i = 0; i < heaters.length; i++) {
             let heater = heaters[i];
@@ -2710,6 +2742,9 @@ class TouchHeaterCommands extends HeaterCommands {
             }
             let type = types.find(elem => elem.val === heater.type);
             if (typeof type !== 'undefined') {
+                // Skip OCP ghost heaters only when there is a matching NCP-controlled
+                // ultratemp/heatpump for this heater's body.
+                if (heater.master === 0 && (type.name === 'hybrid' || type.name === 'ultratemp' || type.name === 'solar') && hasNcpUltratempForBody(heater)) continue;
                 if (inst[type.name] === 'undefined') inst[type.name] = 0;
                 inst[type.name] = inst[type.name] + 1;
                 if (heater.coolingEnabled === true && type.hasCoolSetpoint === true) inst['hasCoolSetpoint'] = true;
@@ -2972,6 +3007,10 @@ class TouchHeaterCommands extends HeaterCommands {
                     [21, { name: 'ultratemp', desc: 'Ultratemp Only', hasCoolSetpoint: htypes.hasCoolSetpoint }]
                 ])
             }
+            else if (ultratempInstalled) {
+                sys.board.valueMaps.heatModes.set(1, { name: 'heatpump', desc: 'Heat Pump' });
+                sys.board.valueMaps.heatSources.set(2, { name: 'heatpump', desc: 'Heat Pump', hasCoolSetpoint: htypes.hasCoolSetpoint });
+            }
             else {
                 // only gas
                 sys.board.valueMaps.heatModes.delete(2);

controller/boards/SystemBoard.ts

@@ -4059,7 +4059,7 @@ export class HeaterCommands extends BoardCommands {
     public updateHeaterServices() {
         let htypes = sys.board.heaters.getInstalledHeaterTypes();
         let solarInstalled = htypes.solar > 0;
-        let heatPumpInstalled = htypes.heatpump > 0;
+        let heatPumpInstalled = htypes.heatpump > 0 || htypes.ultratemp > 0;
         let gasHeaterInstalled = htypes.gas > 0;
         if (sys.heaters.length > 0) sys.board.valueMaps.heatSources = new byteValueMap([[0, { name: 'off', desc: 'Off' }]]);
         if (gasHeaterInstalled) sys.board.valueMaps.heatSources.set(3, { name: 'heater', desc: 'Heater' });
@@ -4250,6 +4250,9 @@ export class HeaterCommands extends BoardCommands {
                     // so that if we have a heater preference set up then we do not have to evaluate the other heater.
                     let heaterTypes = sys.board.valueMaps.heaterTypes;
                     bodyHeaters.sort((a, b) => {
+                        // Sort master=1 (NCP-controlled) heaters before master=0 (OCP-controlled)
+                        // so directly controlled heaters get priority over OCP ghosts.
+                        if (a.master !== b.master) return b.master - a.master;
                         if (heaterTypes.transform(a.type).hasPreference) return -1;
                         else if (heaterTypes.transform(b.type).hasPreference) return 1;
                         return 0;
@@ -4327,11 +4330,11 @@ export class HeaterCommands extends BoardCommands {
                                         // This is the default operation on IntelliCenter and it appears to simply not start on the setpoint.  We can do better
                                         // than this by heating 1 degree past the setpoint then applying this rule for 30 minutes.  This allows for a more
                                         // responsive heater.
-                                        // 
+                                        //
                                         // For Ultratemp we need to determine whether the differential temp
                                         // is within range.  The other thing that needs to be calculated here is
                                         // whether Ultratemp can effeciently heat the pool.
-                                        if (mode === 'ultratemp' || mode === 'ultratemppref') {
+                                        if (mode === 'ultratemp' || mode === 'ultratemppref' || mode === 'heatpump' || mode === 'heatpumppref') {
                                             if (hstate.isOn) {
                                                 // For the preference mode we will try to reach the setpoint for a period of time then
                                                 // switch over to the gas heater.  Our algorithm for this is to check the rate of

controller/comms/messages/Messages.ts

@@ -1488,6 +1488,13 @@ export class Response extends OutboundCommon {
         //
         // NOTE: IntelliCenter response matching is handled in the IntelliCenter-specific block below
         // to keep the logic in one place.
+        if (msgOut.protocol === Protocol.Heater) {
+            // Heater protocol: request action 114 → response action 115, etc.
+            // Verify response comes from the heater we addressed.
+            if (msgIn.source !== msgOut.dest || (msgIn.dest !== msgOut.source && msgIn.dest !== 16)) { return false; }
+            if (this.action > 0 && this.action === msgIn.action) return true;
+            return false;
+        }
         //
         // Restore Response-level action matching for non-IntelliCenter protocols (e.g., Hayward).
         // The Hayward Outbound action getter has a known index mismatch (reads source instead of action),
@@ -1497,7 +1504,7 @@ export class Response extends OutboundCommon {
             if (this.action === msgIn.action) return true;
             else return false;
         }
-        if (msgOut.protocol === Protocol.Pump) {
+        else if (msgOut.protocol === Protocol.Pump) {
             switch (msgIn.action) {
                 case 7:
                     // Scenario 1.  Request for pump status.

controller/comms/messages/status/HeaterStateMessage.ts

@@ -18,6 +18,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 import { Inbound, Protocol } from "../Messages";
 import { state, BodyTempState, HeaterState } from "../../../State";
 import { sys, ControllerType, Heater } from "../../../Equipment";
+import { logger } from '../../../../logger/Logger';
 
 export class HeaterStateMessage {
     public static process(msg: Inbound) {
@@ -85,33 +86,44 @@ export class HeaterStateMessage {
         msg.isProcessed = true;
     }
     public static processUltraTempStatus(msg: Inbound) {
-        // RKS: 07-03-21 - We only know byte 2 at this point for Ultratemp for the 115 message we are processing here.  The 
+        // RKS: 07-03-21 - UltraTemp RS-485 protocol reverse engineering notes.
+        // The heat pump communicates via Action 114 (command) / 115 (response) messages.
+        //
+        // Action 115 - inbound response (heat pump -> controller, heartbeat every ~1s)
+        // [165, 0, 16, 112, 115, 10][160, 1, 0, 3, 0, 0, 0, 0, 0, 0][2, 70]
         // byte  description
         // ------------------------------------------------
-        // 0    Unknown (always seems to be 160 for response)
-        // 1    Unknown (always 1)
-        // 2    Current heater status 0=off, 1=heat, 2=cool
-        // 3-9  Unknown
-        
-        // 114 message - outbound response
-        //[165, 0, 112, 16, 114, 10][144, 0, 0, 0, 0, 0, 0, 0, 0, 0][2, 49] // OCP to Heater
+        // 0    Always 160 for response
+        // 1    Always 1
+        // 2    Current heater status: 0=off, 1=heat, 2=cool
+        // 3    Believed to be offset temp
+        // 4-9  Unknown
+        //
+        // Action 114 - outbound command (controller -> heat pump)
+        // [165, 0, 112, 16, 114, 10][144, 0, 0, 0, 0, 0, 0, 0, 0, 0][2, 49]
         // byte  description
         // ------------------------------------------------
-        // 0    Unknown (always seems to be 144 for request)
-        // 1    Current heater status 0=off, 1=heat, 2=cool
-        // 3    Believed to be ofset temp
+        // 0    Always 144 for request
+        // 1    Sets heater mode: 0=off, 1=heat, 2=cool
+        // 3    Believed to be offset temp
         // 4-9  Unknown
-        
-        //   byto 0: always seems to be 144 for outbound
-        //   byte 1: Sets heater mode to 0 = Off 1 = Heat 2 = Cool
-        //[165, 0, 16, 112, 115, 10][160, 1, 0, 3, 0, 0, 0, 0, 0, 0][2, 70] // Heater Reply
         let heater: Heater = sys.heaters.getItemByAddress(msg.source);
+        if (typeof heater === 'undefined' || !heater.isActive) {
+            // Heat pump not configured for this address
+            msg.isProcessed = true;
+            return;
+        }
         let sheater = state.heaters.getItemById(heater.id);
         let byte = msg.extractPayloadByte(2);
+        let prevOn = sheater.isOn;
+        let prevCooling = sheater.isCooling;
         sheater.isOn = byte >= 1;
         sheater.isCooling = byte === 2;
         sheater.commStatus = 0;
         state.equipment.messages.removeItemByCode(`heater:${heater.id}:comms`);
+        if (prevOn !== sheater.isOn || prevCooling !== sheater.isCooling) {
+            logger.info(`UltraTemp heartbeat: src=${msg.source} status=${byte} (${byte === 0 ? 'OFF' : byte === 1 ? 'HEAT' : 'COOL'}) heater=${heater.name}`);
+        }
         msg.isProcessed = true;
     }
     public static processMasterTempStatus(msg: Inbound) {