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

Add HTTP(s) protocol support to Asuswrt #71899

Closed
wants to merge 1 commit into from

Conversation

ollo69
Copy link
Contributor

@ollo69 ollo69 commented May 15, 2022

Proposed change

This PR add the support to HTTP and HTTPS protocols to AsusWRT integration using this library.
HTTP protocol is the same used by the ASUS Router phone app provided by Asus, so potentially all devices that work with the application should be supported. This protocol in my tests is more reliable in device presence detection, and it also have the advantage to require less information for configuration, and therefore simplifies the initial and option config flows.
Sensors for the moment are limited to Download, Upload and related speeds, more sensors are available but I will leave this for future PR.

Support to existing SSH and TELNET protocol is maintained for compatibility, and config flow requests additional parameter in a second step only when one of this protocols are selected.

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)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

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
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@probot-home-assistant
Copy link

Hey there @kennedyshead, mind taking a look at this pull request as it has been labeled with an integration (asuswrt) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

"""Return a dictionary of available sensors for this bridge."""


class AsusWrtLegacyBridge(AsusWrtBridge):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be removed, there is no real point in dividing into 2 versions?

Copy link
Contributor

Choose a reason for hiding this comment

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

And when doing that also remove me as code-owner would be good. (moved on from hass)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My idea was to maintain both considering the variety of routers in circulation. Can we wait for other opinions?

Copy link

@boheme61 boheme61 May 27, 2022

Choose a reason for hiding this comment

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

My idea was to maintain both considering the variety of routers in circulation. Can we wait for other opinions?

... Could't "commend out" or "strike-through" this, but i finally got it up and running, everything works as expected(from what i can see) ... i apologize for my inpatient nature, but im to old to deal with that :)
.........
If you want "assistant" in troubleshooting your "proposed" actions to verify
https://github.com/ollo69/ha_asuswrt_custom/tree/master/custom_components/asuswrt_custom , you better "shoot when the powder is dry" ... im currently without device-tracking and all, as i installed asuswrt_custom, and nothing works, hour ago today i had both ASUSWRT AND AsusRouter integrations working , so i might run out of patience, or time for testing/playing around :)

....

PS: Im still available (for testing purposes) just to justify my conscience :)

Copy link
Contributor Author

@ollo69 ollo69 May 27, 2022

Choose a reason for hiding this comment

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

... Could't "commend out" or "strike-through" this, but i finally got it up and running, everything works as expected(from what i can see) ... i apologize for my inpatient nature, but im to old to deal with that :)

Never mind, good to know that this work for you. Any suggestion that may come using this implementation are welcome👍

Copy link

@boheme61 boheme61 May 28, 2022

Choose a reason for hiding this comment

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

I have just finished "evaluate" Asus_Router-Custom integration, and i fall back to ASUSWRT, Would be great if You could bring this Integration upto-date, and maybe as you mentioned in "#72591" ADD a few more sensors.
What first come to my mind, was (beside WAN_IP-Sensor) also Router-uptime.sensor

HA " UPnP/IGD " Core-Integration, include these 2 sensors, but from a usage-perspective would it be great if we don't have to use 2 different Core-Integrations, to get info's from the Router
Personally im not interested in DL/UL-Total or Speed etc., But essential info and "Tracking" initially, And using only 1 integration for this :)

As of now i have to use ASUSWRT for it's "Tracker" and "Connected Devices" AND UPnP/IGD, for "RouterUptime" and "WAN_IP"

@shmick
Copy link
Contributor

shmick commented Jul 12, 2022

@ollo69 does this PR need further review?

@ollo69
Copy link
Contributor Author

ollo69 commented Jul 12, 2022

PR must be approved (and merged) by HA dev team.

@shmick
Copy link
Contributor

shmick commented Aug 10, 2022

@frenck can this be added into the next minor or major release?

@shmick
Copy link
Contributor

shmick commented Aug 17, 2022

@kennedyshead can you approve / merge this PR if everything looks ok?

@ollo69
Copy link
Contributor Author

ollo69 commented Aug 17, 2022

@shmick,

PR must be reviewed by HA teams member and only them can merge PR in dev branch.
I suppose that review of this integration is taking long time because is a big PR, and review will require time that probably at this moment HA team do not have.
Unfortunately there are no way to split this in smaller PR, so we can only wait and hope that someone will take a look to this.

@shmick
Copy link
Contributor

shmick commented Sep 28, 2022

Pinging @frenck again to see we can get this merged soon

@frenck
Copy link
Member

frenck commented Sep 28, 2022

Please don't ping for reviews. As a matter of fact, I personally skip PRs asking for it (to prevent pinging from actually working).

There is a large queue of PRs to review. If you want things to go faster, please, join in helping out by reviewing the queues.

../Frenck

@ollo69 ollo69 force-pushed the asuswrt-http-protocol branch 3 times, most recently from 220d760 to 4d00beb Compare December 2, 2022 16:04
@ollo69 ollo69 force-pushed the asuswrt-http-protocol branch 3 times, most recently from 8b12bf1 to f997aaf Compare December 17, 2022 09:03
@ollo69 ollo69 mentioned this pull request Dec 17, 2022
19 tasks
@ollo69
Copy link
Contributor Author

ollo69 commented Dec 31, 2022

I put this PR in draft waiting for review of PR #84152

@ollo69 ollo69 marked this pull request as draft December 31, 2022 14:53
@ollo69 ollo69 mentioned this pull request Apr 11, 2023
19 tasks
@ollo69
Copy link
Contributor Author

ollo69 commented Jul 2, 2023

I close this PR, replaced by PR #95720

@ollo69 ollo69 closed this Jul 2, 2023
@ollo69 ollo69 deleted the asuswrt-http-protocol branch July 2, 2023 16:29
@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants