Add diagnostics to modbus (Satisfy quality demand)

[janiversen]

Breaking change

Proposed change

This add diagnostic information to modbus as requested in #117576.

As per home-assistant/developers.home-assistant#1512, gold and platinum integrations should implement diagnostic ! (modbus is platinum).

The modbus protocol do not offer diagnostic information, so this PR simply returns the current data (something which
is already available in the UI for each entity).

The diagnostic information does not replace the need for a log with debug information, when creating an issue !!

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 Ruff (ruff format 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.

[joostlek]

Again, modbus doesn't use config flow, thus doesn't have a config entry, so this change doesn't make sense. Modbus is a special case because it can be one platinum without having config flow as it is a protocol integration. Thus the requirement don't fully apply, so we either need to make an exception for modbus or rephrase it (but how much effort do you want to put into wording since only mqtt and modbus fall under this rule)

[epenet]

Maybe the solution is to simply add a comment to script/hassfest/manifest.py indicating why it is NOT possible to add it for modbus.
I think the lack of config flow / config entry justifies that.

[janiversen]

And again, integrations without config entries are not excluded.

It is not only modbus, but configurations that do not use config_flow in general.

Apart from that it can actually be used in modbus if the end-user configures the parameter unique id.

[joostlek]

Integrations without config flow are excluded, because they can't get past silver. So they never have to encounter this requirement in the first place.

[janiversen]

in general that is true, but we have at least one exception, and changing the doc or manifest.py could solve that.

or accept the PR even though it's a bit silly.

[epenet]

in general that is true, but we have at least one exception, and changing the doc or manifest.py could solve that.

or accept the PR even though it's a bit silly.

It's not possible to accept the PR as it is... it is failing hassfest!
diagnostics also requires 100% coverage (like config_flow)

[janiversen]

Right now modbus will never have a green CI and thus for all practical purposes is dead.

An alternative (although not very nice) is to remove the modbus integration, with the reason that new PRs are not possible and assume it reappears as a custom component.

Degrading to silver, would have the negative effect, that it would block a custom component since it would fix the pymodbus version.

Please advice which road I as the active maintainer should choose.

[epenet]

Thus my suggestion: add a comment in script/hassfest/manifest.py, next to the modbus entry, explaining why it cannot implement diagnostic.

[janiversen]

That is a good solution, that file as far as I can see, belong to the core team (who added modbus in the first place). So I assume the correct procedure is to close this PR and hope manifest.py gets corrected.

The reason this PR fails is because I did not remove modbus from the no_diagnostic list, which of course should be done with this PR (will do it later today).

[janiversen]

manifest is updated, so hopefully CI goes green.

WIth this PR, the “demand” in #117576 is full filled.

Since modbus was added to manifest.py, and the text do not exclude modbus, I cannot “just” comment modbus out with a comment.

Please remember this PR actually works, if the user uses the parameter “unique id”.

[joostlek]

Please check Discord