Fix slave id equal to 0

[crug80]

Breaking change

Possible breaking change:
The configuration where the slave id or the device_address was not set, now assume a default value equal to 1 (instead of 0).
The situation can be easily fixed by an explicit declaration of the parameter.
Please note that the current bug has already broken those situations.

Proposed change

After having bumped pymodbus 3.7.x an hidden bug belonging the case where the slave id is set to 0 or not set at all became visible.
Before of that change, the underlying library pymodbus in case of missing slave id used a default value equal to 0. From the version 3.7.x, pymodbus changed the defaul slave it to 1, enforcing the fact that the slave id 0 is defined as broadcast according with the ModBus protocol.
This change within the library highlighted a previously hidden bug in the integration where a condition to check if the slave ID was None, actually intercepted even a slave id set to 0,
Fixing the bug, we align also the integration with the Modbus spec and with the pymodbus library setting the default value for the slave id to 1, instead of 0.

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 Modbus Integration Error for Slave ID 0 After Upgrade to Home Assistant 2025.1 #134665
• 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.

To help with the load of incoming pull requests:

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

[frenck]

Can we add a test for this case?

../Frenck

[bdraco]

Looks like the test was accidentally reverted in 8ac274f

[crug80]

Looks like the test was accidentally reverted in 8ac274f

The test was based on the extra_attribute, so that test is not valid anymore after removing that attribute.

[bdraco]

Looks like the test was accidentally reverted in 8ac274f

The test was based on the extra_attribute, so that test is not valid anymore after removing that attribute.

Got it. We should still be able to patch the lib to see whats its sending though instead of verifying the attribute

[bdraco]

I'm a little behind here, but I think we don't need the milestone unless pymodus was already bumped in 2025.1

@crug80 is this issue only present on dev or does it affect stable?

[frenck]

I'm a little behind here, but I think we don't need the milestone unless pymodus was already bumped in 2025.1

@crug80 is this issue only present on dev or does it affect stable?

The linked issue report was against Home Assistant 2025.1, hence me considering this an patch worthy issue.

Ref: #134665

If unsure, we can wait, we are close to beta.

[bdraco]

Chatted with @crug80

It looks like we need to adjust flexit with the same/similar change as well since it uses the modbus code path and it already has the issue since we bumped to pymodbus 3.8.3, however that can be a separate PR and this one should be ok to tag for a patch.

core/homeassistant/components/flexit/climate.py

Line 42 in cd16a57

vol.Required(CONF_SLAVE): vol.All(int, vol.Range(min=0, max=32)),

core/homeassistant/components/flexit/climate.py

Line 57 in cd16a57

modbus_slave = config.get(CONF_SLAVE)

[crug80]

Chatted with @crug80

It looks like we need to adjust flexit with the same/similar change as well since it uses the modbus code path and it already has the issue since we bumped to pymodbus 3.8.3, however that can be a separate PR and this one should be ok to tag for a patch.

core/homeassistant/components/flexit/climate.py

Line 42 in cd16a57

vol.Required(CONF_SLAVE): vol.All(int, vol.Range(min=0, max=32)),

core/homeassistant/components/flexit/climate.py

Line 57 in cd16a57

modbus_slave = config.get(CONF_SLAVE)

Flexit is ok. CONF_ADDRESS is Required and it can assume only values from 0 to 32, so with this fix on modbus we made safe also for flexit use the 0 as slave.

[bdraco]

FAILED tests/components/emulated_roku/test_binding.py::test_events_fired_properly - AssertionError: assert 'keypress' == 'keyup'

Test failure is unrelated

[frenck]

Thanks both!

I'll ship this one tomorrow.

../Frenck