tagyoureit / nodejs-poolController

An application to control pool equipment from various manufacturers.
GNU Affero General Public License v3.0
323 stars 94 forks source link

i10D personality module not identified correctly #207

Closed george-basco closed 3 years ago

george-basco commented 4 years ago

Describe the bug I have an Intellicenter with the i10D (523029Z) module. It is incorrectly being identified as a 522039 cover module. The system is being identified as an i5P.

Expected behavior initExpansionModules is being called with: ocp0A=0x87, ocp0B=0, ocp1A=0, ocp2A=0, ocp3A=0

The slot1 valve expansion board gets identified correctly. slot0 (value of 7) should be identified as i10D but is not.

I changed the code in IntellicenterBoard.ts so that a value of 7 identifies the i10D expansion board. And also changed the line: if (pb.type === 0 || pb.type > 5) that identifies the system as i5P to: if (pb.type === 0 || pb.type > 7)

I then get correct identification in poolConfig.json, but the rest of the config isn't being filled out, e.g. circuits. I'm looking at this code for the first time, so don't understand at all what's going on.

Let me know what information would be helpful. Thanks..

Pool Equipment

george-basco commented 3 years ago

Haven't configured heaters yet, so trying to decipher this stuff.

On the web interface I see exactly what you show above. Spa Manual Heat option only shows for the Spa body. However, if I try to set the option, it doesn't stick. Maybe this is because something isn't configured elsewhere yet?

On the IOS app and OCP, there is no such option on the Bodies configuration pages. Given that it doesn't seem to do anything on the web interface, makes sense that it doesn't show on OCP or IOS app.

rstrouse commented 3 years ago

In the options section there is also a "Manual Heat" setting. Perhaps it has something to do with that. The options is bizarre at best but it will tell us whether it actually treats the dual body system as a "Spa" or not. Right now it looks like you can configure either body to be the pool or you could configure it to be 2 pools.

george-basco commented 3 years ago

I turned on the "Manual Heat" option under "General Settings -> Advanced Settings". This didn't change anything. I still don't see a "Spa Manual Heat" option under the body configurations page in either the OCP or IOS app interface. The option does show up in the web interface, but doesn't seem to do anything as I noted before.

Via the "installation setup" on the OCP, I can set the body type. In this case I made both of them type "spa" image

Or both type "pool" image

rstrouse commented 3 years ago

Omg, there is a body type setting. Shoot a screenshot of that. It appears that the manual heat setting is dependent on this body type and that is the determination for the pool or spa or pool, pool or spa spa aliases for the body attachment.

george-basco commented 3 years ago

Is this what you want?

IMG_0602 IMG_0599 IMG_0600 IMG_0601

rstrouse commented 3 years ago

Well look at that. I had no idea. I am scraping those bytes and didn't do anything with them. I had just assumed that the body type would be pool for both bodies in an i10D.

rstrouse commented 3 years ago

Alright I added support for body types. Turns out I can do this with an i10PS as well. We almost had this correct. Do a pull then reload your config so it picks up the type descriptions.

george-basco commented 3 years ago

Sorry if I'm missing it, but don't see your new changes.

rstrouse commented 3 years ago

Sorry I didn't hit the push. It was waiting for me. It's up now.

george-basco commented 3 years ago

Ok, bodies are getting displayed correctly now. When playing around with the heater settings I noticed an odd behavior. If I change the "Body" setting for a heater in dashPanel, the change does get made (OCP sees it), but dashPanel reverts back to the previous setting.

Here's an example. The starting point is to have two heaters, "gas pool" has body = pool, and "gas spa" has body = spa: image image

This is the starting point for the replay capture.

After the reload completes, I change "gas pool" body to spa, and "gas spa" body to pool. When I click the "save heater" button to save the config, the body reverts back to previous state. This happens for both saves. However, OCP and web interface both register the changes as expected: image image

So making the change through dashPanel is working correctly, but dashPanel state doesn't reflect it. If I "reload config", the state will show correctly to match what OCP and web interface are showing.

One additional oddity is that both OCP and web interface don't allow the body to be changed. They show the second body but greyed out. The IOS app does allow the change to happen like dashPanel.

Here's what the web interface looks like, showing the greyed out pool option: image

Not an important action to begin with and given all the strangeness in behaviors, can't imagine any of this matters. But just wanted to point it out.

Here is the replay that consists of the reload and the two body changes: replay.zip

rstrouse commented 3 years ago

I'll have a look at the replay. I don't experience the same results so it could be the 168 messages coming out of series and we are picking them up as gospel. It is odd that sometimes the web app just broadcasts its current settings after another controller changes them. I have seen the wireless remote then have to ask for the whole series over again. This would that our response includes the goofy settings from the web app and the final results haven't been tabulate it.

Maybe it needs a bit of that or it could be sumpin simpler like we only store it if it is a single or shared system. I'll have a look.

rstrouse commented 3 years ago

Well that was none of the above. Turns out it was reliant on the controllers on the bus to ask for the body setting. In my case the wireless controller seems to always chime in. Anyhow it now persists the value without the assistance of any other controllers on the bus.

tagyoureit commented 3 years ago

Will this cause a problem with findByObject if a users sends a PUT /state/body/heatMode (or setPoint, etc) with {name: 'pool', heatMode: 'heater'} and both body types are set as pool? Guess the user would need to not have customized the name of the body as well.

Maybe just an edge case and the answer is "don't do that" but was thinking about it as I was reading this thread.

rstrouse commented 3 years ago

No the underlying code is still mapped to pool/spa for shared systems. The desc field is what changes. The one weirdo thing is that pool and spa are misnomers they really are body 1 and body 2. They can be flipped around in IntelliCenter and you can have two pools or two spas on a PS or D model. Is it an overall problem? Probably not since the identifiers are really pointing at body 1-4. For those who are calling "pool" or "spa" (not me either) they will always get body1 when sending pool or body2 when sending "spa." Not sure if you can flip these or set it up that way in IntelliTouch.

The identifiers are important though cuz when mapping a heater to the body is what determines which J connector is tripped to turn on the specified heater.

rstrouse commented 3 years ago

Oh one more thing. On shared systems there is a fourth options that maps to body1 or body2. If you have them mapped to Pool/Pool or Spa/Spa that is how it appears. If you have it set up like the default it is Pool/Spa. These are equipment mappings which show which body or bodies the equipment is available on. This is true for chlorinators, chem controllers, and intake/return valves as well.

george-basco commented 3 years ago

Options for a Heat Pump don't exist in the other interfaces. Adding a heat pump via OCP or web interface doesn't offer any additional options. When viewed in dashPanel, the options below are shown but cannot be set without error.

image image

rstrouse commented 3 years ago

Actually, these settings did exist for a Heat Pump on the web app at one time. However, they removed the heat pump from the selection recently and added hybrid. What is really odd about this is that there is a different list on the Mobile App. Also if you look at the connections these are addressed on the RS485 bus. What is really odd is that the mobile app uses a different type id than the web app. Do you have a heat pump wired into your system? I am looking for someone who has one of these to ferret it all out.

george-basco commented 3 years ago

I don't currently have a heat pump, although I'm pretty sure I'll be adding one at some point.

FYI, I see the same set of heater types in dashPanel, interllicenter.com web interface, and IOS app: Gas, Solar, UltraTemp, MasterTemp, MAX-E Therm, Hybrid, Heat Pump. All three interfaces show the same 7 types.

However, OCP and ICP (running 1.040) are only showing Gas, Solar, Heat Pump, UltraTemp, Hybrid.

I did see all 7 on ICP when it was running 1.045.

Anyway, given that Pentair can't be consistent and that these are one time setup issues, obviously not a big deal.

rstrouse commented 3 years ago

Yeah it will b good 2 have the actual equipment. I also have a feeling that the heaters will need a revisit if they ever release 1.045 or 1.047

george-basco commented 3 years ago

Water Sensor 2 is not being read. Looks like there is some dual equipment logic needed around here:

https://github.com/tagyoureit/nodejs-poolController/blob/f0ef553edeb089e8acaff3b8f80906d9ee0b3376/controller/comms/messages/status/EquipmentStateMessage.ts#L399

In my case byte 15 does have the Water Sensor 2 data. Haven't looked at the solar sensors..

rstrouse commented 3 years ago

Pull Next, this should be fixed. It was previously only looking for expansion panels to populate this value. It now checks to see if we have a shared or dual system as well. waterSensor2 is not valid in shared or single body systems without an expansion.

george-basco commented 3 years ago

With 1.047 firmware released, I looked at differences between the various interfaces. All the interfaces show the same 7 types of heaters. Comparing OCP running 1.047, dashPanel (latest -next), Web Client Version: 1.048 b8 Jun 9, 2020, IOS app 2.0.133

Gas: OCP and IOS app have "Cooldown Delay" option; dashPanel and web do not; Solar: same for all 4 interfaces Heat Pump: dashPanel has options for "Address", "Differential Temp", and "Nocturnal Cooling. OCP, IOS app, and web have none of these UltraTemp: OCP has option for "Cooling Enabled"; dashPanel shows "Differential Temp" and "Nocturnal Cooling"; IOS app and web have "Differential Cooling" and "Cooling Enabled" MasterTemp: same Max E-Therm: same Hybrid: same except IOS app adds a "Heat Mode" option

george-basco commented 3 years ago

I noticed sys.general.options.pumpDelay is not getting read correctly on a reload. https://github.com/tagyoureit/nodejs-poolController/blob/41ab9efb0eb275bd97edf59d56d315f86c5438ba/controller/comms/messages/config/OptionsMessage.ts#L46

instead of extracting byte 37, it should be 30. This seems to fix it on my system and matches what's here: https://github.com/tagyoureit/nodejs-poolController/blob/41ab9efb0eb275bd97edf59d56d315f86c5438ba/controller/boards/IntelliCenterBoard.ts#L931-L934

It also looks like this code is not correct: https://github.com/tagyoureit/nodejs-poolController/blob/41ab9efb0eb275bd97edf59d56d315f86c5438ba/controller/boards/IntelliCenterBoard.ts#L963-L966 I think the numbers 36,39 should be 37,40?

This time to match what's here: https://github.com/tagyoureit/nodejs-poolController/blob/41ab9efb0eb275bd97edf59d56d315f86c5438ba/controller/comms/messages/config/OptionsMessage.ts#L48

I also was able to verify this change does seem to work correctly on my system..

george-basco commented 3 years ago

Haven't configured heaters yet, so trying to decipher this stuff.

On the web interface I see exactly what you show above. Spa Manual Heat option only shows for the Spa body. However, if I try to set the option, it doesn't stick. Maybe this is because something isn't configured elsewhere yet?

On the IOS app and OCP, there is no such option on the Bodies configuration pages. Given that it doesn't seem to do anything on the web interface, makes sense that it doesn't show on OCP or IOS app.

Btw, I think I'm a bit less confused about the Spa Manual Heat option. Don't know if it's because of my dual equipment config, but this option no longer seems to be a per-body option and has moved to a global option under "General Settings -> Advanced Settings" (on web interface). dashPanel should add this to the general settings tab. Might need to change the name of "delays" or split it out with the manual op priority option.

Currently dashPanel shows "spa manual heat" option for spa type bodies under bodies tab. The setting does seem to stick, once you set it, but it has no effect. OCP and IOS app do not show a per-body option. The web interface does still show it, but the option doesn't work, so guessing a bug that it shows up at all.

george-basco commented 3 years ago

The i10D, like the i10PS, should have 11 circuits. 9 AUX + pool + spa. Current definition has 10.

https://github.com/tagyoureit/nodejs-poolController/blob/41ab9efb0eb275bd97edf59d56d315f86c5438ba/controller/boards/IntelliCenterBoard.ts#L111

rstrouse commented 3 years ago

@george-basco,

I posted changes to support the 1.047 firmware. You will need both njspc and -dashPanel.

george-basco commented 3 years ago

Part of https://github.com/tagyoureit/nodejs-poolController/issues/207#issuecomment-694510153 is still a problem:

It also looks like this code is not correct: https://github.com/tagyoureit/nodejs-poolController/blob/41ab9efb0eb275bd97edf59d56d315f86c5438ba/controller/boards/IntelliCenterBoard.ts#L963-L966

I think the numbers 36,39 should be 37,40? This time to match what's here: https://github.com/tagyoureit/nodejs-poolController/blob/41ab9efb0eb275bd97edf59d56d315f86c5438ba/controller/comms/messages/config/OptionsMessage.ts#L48

I also was able to verify this change does seem to work correctly on my system..

rstrouse commented 3 years ago

Funny, I documented it that way originally then probably looked at the 30 message documentation to set the 168 command.

rstrouse commented 3 years ago

I am closing this open a new issue if there are different problems.