Skip to content

Add compensation integration#15192

Merged
frenck merged 12 commits intohome-assistant:nextfrom
Petro31:next
Apr 4, 2021
Merged

Add compensation integration#15192
frenck merged 12 commits intohome-assistant:nextfrom
Petro31:next

Conversation

@Petro31
Copy link
Copy Markdown
Contributor

@Petro31 Petro31 commented Oct 11, 2020

Adds the compensation integration

Proposed change

Adds the compensation integration documentation

Type of change

  • Spelling, grammar or other readability improvements (current branch).
  • Adjusted missing or incorrect information in the current documentation (current branch).
  • Added documentation for a new integration I'm adding to Home Assistant (next branch).
  • Added documentation for a new feature I'm adding to Home Assistant (next branch).
  • Removed stale or deprecated documentation.

Additional information

Checklist

  • This PR uses the correct branch, based on one of the following:
    • I made a change to the existing documentation and used the current branch.
    • I made a change that is related to an upcoming version of Home Assistant and used the next branch.
  • The documentation follows the Home Assistant documentation standards.

Adds the compensation integration
@probot-home-assistant probot-home-assistant Bot added in-progress This PR/Issue is currently being worked on needs-rebase The PR has been branched of the wrong base branch or targets an incorrect target branch labels Oct 11, 2020
@probot-home-assistant probot-home-assistant Bot added next This PR goes into the next branch Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! has-parent This PR has a parent PR in another repo labels Oct 11, 2020
@klaasnicolaas klaasnicolaas added new-integration This PR adds documentation for a new Home Assistant integration and removed in-progress This PR/Issue is currently being worked on needs-rebase The PR has been branched of the wrong base branch or targets an incorrect target branch labels Oct 11, 2020
Comment thread source/_integrations/compensation.markdown Outdated
Co-authored-by: Klaas Schoute <klaas_schoute@hotmail.com>
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @Petro31.

I've reviewed it, and left some comments. Could you please take a look?

Thanks 👍

Comment thread source/_integrations/compensation.markdown Outdated
Comment thread source/_integrations/compensation.markdown Outdated
Comment thread source/_integrations/compensation.markdown Outdated
Comment thread source/_integrations/compensation.markdown Outdated
Comment thread source/_integrations/compensation.markdown Outdated
Comment thread source/_integrations/compensation.markdown Outdated
Petro31 and others added 5 commits October 12, 2020 11:25
Co-authored-by: Franck Nijhof <frenck@frenck.nl>
Co-authored-by: Franck Nijhof <frenck@frenck.nl>
Co-authored-by: Franck Nijhof <frenck@frenck.nl>
Updated yaml example to account for ADR-0007.
Added quotes.
@MatthewFlamm
Copy link
Copy Markdown
Contributor

MatthewFlamm commented Oct 13, 2020

I think it is worth clearly stating that this integration does not interpolate. It fits a single line (or higher order polynomial) to all the data points. I think this will cause confusion as it does with the similar esphome usage. For example:

# Example configuration.yaml entry
compensation:
  media_player_db_volume:
    entity_id: media_player.yamaha_receiver
    data_points:
      - 0.2 -> -80.0
      - 0.5 -> -20.0
      - 1.0 -> -10.0

A value of 0.75 will not transform to -15.0. In fact a value of 0.5 won't transform to -20 in this case either!

@Petro31
Copy link
Copy Markdown
Contributor Author

Petro31 commented Oct 14, 2020

I think it is worth clearly stating that this integration does not interpolate. It fits a single line (or higher order polynomial) to all the data points. I think this will cause confusion as it does with the similar esphome usage. For example:

# Example configuration.yaml entry
compensation:
  media_player_db_volume:
    entity_id: media_player.yamaha_receiver
    data_points:
      - 0.2 -> -80.0
      - 0.5 -> -20.0
      - 1.0 -> -10.0

A value of 0.75 will not transform to -15.0. In fact a value of 0.5 won't transform to -20 in this case either!

@MatthewFlamm It's funny you should mention that. I was planning on adding non-linear interpolation at some point down the road. Does the ESPHome version do interpolation?

@MatthewFlamm
Copy link
Copy Markdown
Contributor

MatthewFlamm commented Oct 14, 2020

I am only aware of the calibrate_linear (and calibrate_polynomial) filter in esphome that works like this integration. I think an interpolate integration would also be useful, but this seems off topic. My suggestion here is to add something like "A single polynomial, linear by default, is fit to all data points provided."

@Petro31
Copy link
Copy Markdown
Contributor Author

Petro31 commented Oct 14, 2020

I am only aware of the calibrate_linear (and calibrate_polynomial) filter in esphome that works like this integration. I think an interpolate integration would also be useful, but this seems off topic. My suggestion here is to add something like "A single polynomial, linear by default, is fit to all data points provided."

I'm not the greatest tech writer, mind if I use that line?

@MatthewFlamm
Copy link
Copy Markdown
Contributor

I'm not the greatest tech writer, mind if I use that line?

You can use it.

frenck
frenck previously approved these changes Oct 15, 2020
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved. Can be merged as soon as the parent PR gets merged.

Clarify that this is a polyfit, not any other type of fits.
@Petro31 Petro31 requested a review from klaasnicolaas October 15, 2020 20:29
@frenck frenck added the awaits-parent Awaits the merge of an parent PR label Oct 20, 2020
frenck
frenck previously approved these changes Oct 20, 2020
Comment thread source/_integrations/compensation.markdown Outdated
@MartinHjelmare MartinHjelmare added parent-merged The parent PR has been merged already and removed awaits-parent Awaits the merge of an parent PR labels Apr 3, 2021
Comment thread source/_integrations/compensation.markdown Outdated
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 @Petro31 👍

@frenck frenck merged commit a6662ae into home-assistant:next Apr 4, 2021
@probot-home-assistant probot-home-assistant Bot removed the parent-merged The parent PR has been merged already label Apr 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! hacktoberfest-accepted has-parent This PR has a parent PR in another repo new-integration This PR adds documentation for a new Home Assistant integration next This PR goes into the next branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants