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

Adds new integration [amitfin/patch] #2103

Closed
wants to merge 2 commits into from

Conversation

amitfin
Copy link
Contributor

@amitfin amitfin commented Oct 10, 2023

Checklist

  • I've read the publishing documentation.
  • I've added the HACS action to my repository.
  • (For integrations only) I've added the hassfest action to my repository.
  • The actions are passing without any disabled checks in my repository.
  • I've added a link to the action run on my repository below in the links section.
  • I've created a new release of the repository after the validation actions were run successfully.

Links

Link to current release: https://github.com/amitfin/patch/releases/tag/v1.1.1
Link to successful HACS action (without the ignore key): https://github.com/amitfin/patch/actions/runs/6463495942/job/17546742448
Link to successful hassfest action (if integration): https://github.com/amitfin/patch/actions/runs/6463495942/job/17546742580

@ludeeus
Copy link
Member

ludeeus commented Jan 7, 2024

I'm not going to allow this.
https://hacs.xyz/docs/publish/include

Custom integrations that override core integrations will not be accepted, you can still use it as a custom repository

Not only will this be used to override core integrations, it will also be done persistently without any possibility of detection, or warning. This will also bypass safe mode (which is designed to start core without any custom modifications).

For libraries I suggest https://developers.home-assistant.io/docs/creating_integration_manifest?_highlight=deps#custom-requirements-during-development--testing instead, as for core integration modifications you want to try, you can use custom components.

@ludeeus ludeeus closed this Jan 7, 2024
@amitfin
Copy link
Contributor Author

amitfin commented Jan 7, 2024

Ok, fair enough.

Just for the records, this integration was developed for solving a real blocker:

  • We have Pima Force 144 alarm system.
  • SIA built-in integration supports its protocol, but requires handling 2 additional op codes.
  • A straightforward PR was created for enhancing the supporting library (pysiaalarm) with the additional 2 op codes. It's still waiting to be reviewed (or noticed). The PR was created on 12/Sep/2023, so it's more than 100 days old by now.
  • This is not a testing environment. We use the integration in many home automation scenarios by now.
  • Without this (Patch) integration there was no way to connect the alarm system to HA until the PR is accepted (which can be months or years from now).

Anyway, as mentioned above, there is no real need to add the integration to HACS. It's still possible to use HACS as the installer via custom repository.

@amitfin
Copy link
Contributor Author

amitfin commented Jan 7, 2024

BTW, I'll be glad to learn if there is a better solution for this blocker...

@ludeeus
Copy link
Member

ludeeus commented Jan 7, 2024

The link to the developer docs I shared can be used for that.
Instead of a version, you can tell it to install from a git branch (explained other places in the dev docs (on mobile now so not easy to look up) I have used that way myself a few times)

@amitfin
Copy link
Contributor Author

amitfin commented Jan 7, 2024

This is a production environment of HAOS on RPI4.
Is it possible to add switches to the hass command in this environment? (--skip-pip-packages pysiaalarm)

@ludeeus
Copy link
Member

ludeeus commented Jan 7, 2024

I do not think so, but should not be needed.
That's mainly to prevent conflicts in venvs, for the container installs (all but core) the libs are already present.

You may need to install it to the root of your config instead of in the deps directory inside it.
Can't look up exactly how the loading order is.

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.

2 participants