Add raid array degraded state binary sensor to freebox sensors

[fthiery]

Proposed change

I use the Freebox router for hosting my HA setup, with a RAID setup (the router has 4 SATA bays with sw RAID functionality and provides virtualization support). Unfortunately their product has no notification feature to report that the array is degraded (i discovered mine was degraded after a power loss event by chance), so i want to add this property to the current Freebox component, e.g. mostly to add a notification.

This PR basically creates one binary_sensor.freebox_server_r2_raid_array_0_degraded-like boolan sensor entity per RAID array in diagnostics entities.

• This is my first attempt at contributing (so, probably not stellar ;), sorry about that.
• For consistency reasons, i overwrote the original author disk mock data

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: freebox: add raid array health binary sensor (#95242) home-assistant.io#28080

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][dev-checklist]
• I have followed the [perfect PR recommendations][perfect-pr]
• The code has been formatted using Black (black --fast homeassistant tests)
• Tests have been added to verify that the new code works.

[home-assistant]

Hi @fthiery

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[home-assistant]

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

Code owner commands

Code owners of freebox 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 freebox Removes the current integration label and assignees on the pull request, add the integration domain after the command.

[jbouwh]

Please add some tests specific for your feature. Not just change testdata.

[home-assistant]

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

Learn more about our pull request process.

[fthiery]

Please add some tests specific for your feature. Not just change testdata.

I added a test, please take a look

[fthiery]

For the record, after applying the requested changes, i deployed this as custom_component and i'm having no issues

[jbouwh]

Sorry accidentally rebased your branch. Can you try to add a test for the code that is not covered yet?
Also have a look at the _attr_name. May be this can set to None so we get translations. We should set _attr_has_entity_name to True.

[fthiery]

Sorry accidentally rebased your branch.

NP, i repushed

Can you try to add a test for the code that is not covered yet?

What are you referring to, all other sensors that were previously here and had zero tests ? Please be more specific.

Also have a look at the _attr_name. May be this can set to None so we get translations.

Well, i think we should not do that, because a Freebox can have multiple raid arrays, so the name should really include the raid array number.

Similar to the disk sensor: https://github.com/home-assistant/core/blob/dev/homeassistant/components/freebox/sensor.py#L191

What do you think ?

We should set _attr_has_entity_name to True.

Added

[jbouwh]

What are you referring to, all other sensors that were previously here and had zero tests ? Please be more specific.

See CI tests. Since the new code has tests now, we need to test all cases.

[fthiery]

What are you referring to, all other sensors that were previously here and had zero tests ? Please be more specific.

See CI tests. Since the new code has tests now, we need to test all cases.

Are you asking me to test more cases on the specific feature i added (e.g. mocking a situation where the raid array gets degraded) or to test the previously existing sensors ?

[jbouwh]

What are you referring to, all other sensors that were previously here and had zero tests ? Please be more specific.

See CI tests. Since the new code has tests now, we need to test all cases.

Are you asking me to test more cases on the specific feature i added (e.g. mocking a situation where the raid array gets degraded) or to test the previously existing sensors ?

Not in this PR, but it would be nice if you did so. BTW the existing modules were listed in .coveragerc, hence they are allowed not to have tests.

These files are in .coveragerc:

    homeassistant/components/freebox/camera.py
    homeassistant/components/freebox/device_tracker.py
    homeassistant/components/freebox/home_base.py
    homeassistant/components/freebox/router.py
    homeassistant/components/freebox/sensor.py
    homeassistant/components/freebox/switch.py

[jbouwh]

Added some suggestions to let the CI tests pass and to test the sensor in a better way

[jbouwh]

It seems you still have to update the docs: https://www.home-assistant.io/integrations/freebox/

Can you open a docs PR, keep in mind you target the PR to the next branch.

[jbouwh]

Needs a docs update, PR looks good,
Thnx @fthiery 👍

[fthiery]

Needs a docs update, PR looks good, Thnx @fthiery +1

home-assistant/home-assistant.io#28080