Add JVC Projector Sensors

[msavazzi]

Breaking change

Proposed change

Add JVC Projector Sensors, Power Status and Input

Type of change

• [ ] Dependency upgrade
• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New integration (thank you!)
• [x] 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: https://github.com/home-assistant/home-assistant.io/pull/31077

Checklist

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

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

• [x] 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:

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

[home-assistant[bot]]

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

Code owner commands

Code owners of jvc_projector 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 jvc_projector Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[msavazzi]

And tests are failing

Now they are all green 😀

[SteveEasley]

@msavazzi Looks like you are pointing to the wrong documentation PR.

[SteveEasley]

The documentation that was merged only talks about the binary sensory that was added a few weeks ago (https://github.com/home-assistant/home-assistant.io/pull/31058/files). I see nothing about the new sensors being added in this PR (https://github.com/home-assistant/home-assistant.io/blob/next/source/_integrations/jvc_projector.markdown).

[msavazzi]

The documentation that was merged only talks about the binary sensory that was added a few weeks ago (https://github.com/home-assistant/home-assistant.io/pull/31058/files). I see nothing about the new sensors being added in this PR (https://github.com/home-assistant/home-assistant.io/blob/next/source/_integrations/jvc_projector.markdown).

Fixed the documentation PR, do not know what's happened but the file was not pushed. Now should be fine

[edenhaus]

I need a second opinion on https://github.com/home-assistant/core/pull/108949#discussion_r1486174506 as in my opinion this should be a select entity

[SteveEasley]

imo if we make it a Sensor Entity now, we can't easily add a Select Entity later without making the integration confusing for users (multiple places for the reported state), or breaking the integration by replacing it (the Sensor disappears).

[msavazzi]

imo if we make it a Sensor Entity now, we can't easily add a Select Entity later without making the integration confusing for users (multiple places for the reported state), or breaking the integration by replacing it (the Sensor disappears).

Correct, but the point is not to add a Select Entity as is not how the Projector works :) Projector are:

• Sensor (read only)
• Remote (write)

:)

[SteveEasley]

Correct, but the point is not to add a Select Entity as is not how the Projector works :) Projector are:

• Sensor (read only)
• Remote (write)

The physical remote is only one UI to the projector, the network port is another interface, and its the one HA uses. So we are not limited to having to represent all functionality as if it were simply a remote control.

What you describe above (read only, write) is exactly how the Select Entity works, but in a more HA native way.

SelectEntity::async_update() (for read)
SelectEntity::async_select_option() (for write)

If fact I think you would get quicker feedback on the state change when writing since async_select_option() would instantly update the HA state (to be further confirmed on the next async update).

All this said I did look at at least one other integration (sleepiq) that uses SelectEntity and it does the polling with a SelectEntity specific DataCoordinator. I would hope this could be done by reading the state already learned from the platform entity's DataCoordinator since it reads 3 separate states in one client API call already.

[SteveEasley]

Another way to think about this is that things like:

• power state (warming, cooling, etc)
• lamp time
• color space
• etc

are all read only and make sense as Sensors. The input (hdmi1, hdmi2) is read/write so it makes sense to implement it with an entity type that supports read/write.

[SteveEasley]

@msavazzi I do understand this would be a fair amount of work for you to implement if we do agree this is the correct route, especially since you don’t plan to personally use the integration for writing to the projector. So I would volunteer to implement the change in your branch if you wanted.

[msavazzi]

@msavazzi I do understand this would be a fair amount of work for you to implement if we do agree this is the correct route, especially since you don’t plan to personally use the integration for writing to the projector. So I would volunteer to implement the change in your branch if you wanted.

That would be super great. can we work on the same fork? maybe we can chat on discord (msavazzi) to make this happen :)

[joostlek]

A select entity would be a separate PR. So this PR should not include that sensor.

[msavazzi]

@msavazzi I do understand this would be a fair amount of work for you to implement if we do agree this is the correct route, especially since you don’t plan to personally use the integration for writing to the projector. So I would volunteer to implement the change in your branch if you wanted.

As I started developing in python 2 weeks ago for this component :) any help is welcome. I've added you to my dev branch so we can work on it together.

@joostlek I've amended this PR removing the input so we can merge this PR, then we can create the new one for the Select Entity as you said. Can you merge this in dev?

[msavazzi]

@joostlek @edenhaus @MartinHjelmare can we merge this in to stable? what's missing?