Bump screenlogicpy to v0.9.0

[dieselrabbit]

Breaking change

Some entity names have changed. Integration will migrate old entities to new names/ids, but users may have to manually update old entities used in dashboards.

Invalid entities such as "Saturation Index" are removed if the required equipment is not configured.

Proposed change

• Bump screenlogicpy to v0.9.0.
  dieselrabbit/screenlogicpy@v0.8.2...v0.9.0
• Support major data refactor in api library.
  • Explicitly define supported entities
  • Update screenlogic entities to use EntityDescription.
    • Explicitly define supported data
  • Handle new keys/names from api library that were renamed for clarity and/or accuracy.
    • Migrate old entities to new entity id/names.
    • Remove existing entities missing required equipment
  • Supports new binary_sensor entities
    • ORP HIGH Alarm
    • PH LOW Alarm
    • SI Corrosive
    • SI Scaling

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Black (black --fast homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hey there @bdraco, mind taking a look at this pull request as it has been labeled with an integration (screenlogic) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of screenlogic can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign screenlogic Removes the current integration label and assignees on the pull request, add the integration domain after the command.

[dieselrabbit]

Very much a DRAFT

[dieselrabbit]

Running in production for over a week. No issues. I think it's ready for any further review.

[bdraco]

Those circuits do appear on the intellitouch panel main screen so don't show might not be so useful

[dieselrabbit]

I think that setting may only affect the ScreenLogic app. I'll have to do some more research.

[dieselrabbit]

Confirmed, that setting is specifically labeled for "ScreenLogic Display" and described as such in the ScreenLogic2 User Manual.
This functionality was added in #88518.

[bdraco]

Going to do a delete, revert, re-setup, upgrade later today to see which problems are cruft and what still needs to be addressed (if anything)

[bdraco]

Houston before

[bdraco]

After update

[bdraco]

air blower, cleaner, jets all disappeared

[bdraco]

Confirmed, that setting is specifically labeled for "ScreenLogic Display" and described as such in the ScreenLogic2 User Manual. This functionality was added in #88518.

So what I think happened is since they were already enabled I didn't notice the problem in that PR. I'm only seeing it now since I've removing/re-adding and the entities are disabled by default now.

I'm leaning towards we shouldn't check this flag since it hides entities that are displayed on the main control panel which is not expected and likely not desirable behavior

[dieselrabbit]

I'm a big proponent of the user experience. I've thought about the situation here a lot today. Probably more than I should have. I came to the conclusion that if "Don't Show" was the default setting when first setting up a ScreenLogic system, I'd agree. In this case though, circuits have to be manually set to "Don't Show", indicating it's most likely an intentional configuration and in my opinion should still apply. The configuration app will even reset the Show On setting, removing 'Don't Show' when changing a circuit's function to anything other than "[NOT USED]".

At one point I wondered if I were to re-address this, would entity_registry_visible_default be a better match for the "Don't Show" setting, thinking Don't Show just means it's not on the Pool, Spa, Features, or Lights pages; but in checking the mobile app, I see a circuit set to Don't Show isn't included anywhere, even in the Summary -> Circuits page so, it's not a match to the apps experience. Still that could be a better but not incongruent experience.

[bdraco]

I see jets on the spa section in the app but it's disabled by default with the integration

When I get home Thursday I'll dig into this a bit more.

[dieselrabbit]

That's interesting. I'd be very curious as to what that circuit lists for Show On in the SLConfig app.

[bdraco]

I'm finally back home so I'll do testing this weekend. Sorry for the delay

[bdraco]

Here is the intellitouch control panel (grabbed the remote since its easier to photograph):

Here is the app config:

[bdraco]

Confirmed the disabled entities is an existing issue and not a regression

[bdraco]

Thanks @dieselrabbit

[dieselrabbit]

Thank you @bdraco !

[MartinHjelmare]

I think this PR was too large and looks very high risk for bugs. I think it should have been preceded by PRs that added platform tests for all platforms.