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

Trigger OTA for not initialized devices #485

Conversation

chemelli74
Copy link
Collaborator

Allow OTA to work for devices that are not initialized.
This will be used in HA for repairing an unsupported firmware for connected devices.

@thecode
Copy link
Contributor

thecode commented Jan 18, 2024

Honestly, I think we should drop this, we enforced minimum firmware version to reduce our code maintenance efforts and end up with complicated code to now workaround about firmware update problems

aioshelly/common.py Outdated Show resolved Hide resolved
aioshelly/common.py Outdated Show resolved Hide resolved
aioshelly/common.py Outdated Show resolved Hide resolved
@thecode
Copy link
Contributor

thecode commented Jan 21, 2024

I still think we should not go forward with this, the added complexity doesn't worth the benefit. If we complicated the code so much, I would rather go back to were we was in core 2023.12 before we introduced the minimum firmware requirement (which was meant to reduce code complexity and not increase it.

Auth for Gen2 will not work as implemented here since Gen2 uses digest, if we have to duplicate this logic for HTTP we now increase the code that we have to maintain backward compatibility.

try:
await session.get(
URL.build(scheme="http", host=host, path=f"/{path}"),
auth=auth,
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail if used for Gen2

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To avoid complexity of the code, we should move the Auth management out of the class so to reuse it here and there

Copy link
Contributor

Choose a reason for hiding this comment

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

The auth code is part of a class so you would have to do many changes for this and you end up with a code that was supposed to serve for wsrpc under common while it is nothing in common with Gen1.

I really don't understand were you are going with it. We enforce a minimum version which blocks us from initializing a device (although any minimum you set for Gen2 will not block you from initializing the device since there are no breaking API changes) than we move code to common so we can talk to the device without initializing it while we shouldn't have gone there in the first place.

@chemelli74 chemelli74 force-pushed the chemelli74-indipendent-ota branch from 14eff07 to e8b0fb7 Compare February 3, 2024 16:14
@chemelli74
Copy link
Collaborator Author

Superseded by #544

@chemelli74 chemelli74 closed this Apr 11, 2024
@chemelli74 chemelli74 deleted the chemelli74-indipendent-ota branch April 11, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants