Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🏡 home-assistant: spring cleaning #157213

Merged
merged 15 commits into from
Feb 15, 2022
Merged

🏡 home-assistant: spring cleaning #157213

merged 15 commits into from
Feb 15, 2022

Conversation

mweinelt
Copy link
Member

@mweinelt mweinelt commented Jan 29, 2022

Motivation for this change

Some overdue maintenance tasks and some features all users have been waiting for. 🏡

  • Create a home-automation module category and move home-assistant into it
  • Update default package example with something reasonable that makes onboarding work
  • Clean up old patches
  • Wait for network-online.target
    • I've seen multiple components fail when the network isn't ready yet :(
  • Update module interface
    • rfc42 style configuration
      • use pkgs.formats
    • allow configuring extraComponents and extraPackages from the module
      • This is so we don't require people to overwrite the package themselves, which has a bad discoverability
    • Apply autoExtraComponents unconditionally and remove the option
    • Migrate default configuration (timezone, port, lovelace mode) and remove applyDefaultConfig option

Postponed

  • Allow use of the static files functionality by following symlinks in /var/lib/hass/www (served as /local)
  • Custom lovelace modules
  • Custom components
    • Propagating python dependencies

Open questions:

  • Rename config to settings? And lovelaceConfig to lovelaceSettings?
  • How to create optional mkOption fields? Not well supported, have to all null values and filter them out recursively.
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

# Final list of extra packages passed into the package, that also includes custom
# component dependencies
extraPackages = cfg.extraPackages
++ map (component: component.dependencies or []) cfg.customComponents;
Copy link
Member Author

@mweinelt mweinelt Feb 1, 2022

Choose a reason for hiding this comment

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

We somehow need to propagate python dependencies that custom components have into the final home-assistant package. The silly idea is to define passthru.dependencies, wondering if there is a more established way of handling these kinds of relationships in nixpkgs.

This is sitill untested, as there is no custom component packaged yet, that would require that.

"met"
"esphome"
] ++ optionals (pkgs.stdenv.hostPlatform.isAarch32 || pkgs.stdenv.hostPlatform.isAarch64) [
"rpi_power"
Copy link
Member Author

Choose a reason for hiding this comment

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

Is that the correct condition to match the target platform?

@graham33
Copy link
Contributor

graham33 commented Feb 9, 2022

I dropped custom components and lovelace modules and the strongly related symlink follow patch for now. They are what's keeping this PR from getting merged sooner than later. They are in a local branch and I'll open another draft PR for them soon.

Happy to help on the custom component stuff

@graham33
Copy link
Contributor

I read through the PR and looks good to me. For the custom component stuff, I guess I could start a PR for buildHomeAssistantCustomComponent, since it seems fairly separable from other parts?

@mweinelt
Copy link
Member Author

I looked through your nur repo recently and I liked the bits about parsing the manifest for dependencies, and using propagatedBuildInputs with a custom builder function. I think that is the way to go, but I wouldn't want to tackle it alone. I also looked into pytest-home assistant-custom-component, which is the reason you wanted home-assistant to be importable. It would be nice to have and we should look into getting that to work, likely using toPythonModule.

@mweinelt mweinelt force-pushed the hass-module branch 2 times, most recently from fb189fe to b592410 Compare February 14, 2022 01:52
@mweinelt mweinelt marked this pull request as ready for review February 14, 2022 01:54
@mweinelt
Copy link
Member Author

I have walked through these changes with @maralorn and I'm thankful for this feedback. 🙏

I now consider this set of changes ready for the final review period. ⌛

If you have any input on the changes that are in the given scope right now, post them ASAP.

@mweinelt mweinelt force-pushed the hass-module branch 2 times, most recently from dae77d5 to c5a11cf Compare February 14, 2022 12:13
Putting so many things into misc is not great, so let's open up a new
category called home-automation here and now.
The given example is now closer to a sane default people will want to
start with. It also displays the existance of extraComponents, a feature
that will receive more usage with home-assistant warning about
components that have completely migrated away from YAML configuration.
I doesn't seem to be needed anymore. I built all tests and didn't
experience any failures that could've been prevent by it.
If people take the time to setup network-online.target correctly we
should wait for it. If they don't it's basically the same as
network.target anyway, so no harm done.

Over time I've seen multiple integrations that have dealt badly with
missing network connectivity at startup, this should alleviate further
pains.
mweinelt and others added 10 commits February 15, 2022 23:41
After this change users with non-declarative configs need to set
`services.home-assistant.config` to an `null`, or their
`configuration.yaml` will be overwritten.

The reason for this is that with rfc42 style defaults the config
attribute set will never be empty by default.
It simply should not be required to override the package for such a
common use case, especially since the module usually adds another
override on top to inherit extraComponents.
Database provisioning was shown to be racy since adding a recorder test
using PostgreSQL. There is no harm in waiting for these services,
because if they're not available they will be ignored.
Passing psycopg2 for PostgreSQL support in the recorder component is one
of the more popular use cases to pass an extra package.
The rpi_power integration is part of the onboarding flow on Raspberry Pi
SBCs.
Co-Authored-By: Martin Weinelt <hexa@darmstadt.ccc.de>
There are now multiple combinations of how one can pass either
extraPackages or extraComponents. We now test those passed directly to
the package via an override, and those passed indirectly via the module,
that ultimately results in a second override to the package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants