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 Starlink Integration #77091
Add Starlink Integration #77091
Conversation
I am working on tests now, I'd appreciate review on the code that's there in case there's something significant I've missed :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aa5a80a
to
c45cebb
Compare
If we're happy with the state of the config flow and sensors, I will begin writing some tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sensors look good to me - see some comments though.
Not sure about config flow, I don't know much about it.
d4d8b04
to
ec26e4d
Compare
cf42c4a
to
a0037af
Compare
Co-authored-by: J. Nick Koston <nick@koston.org>
Co-authored-by: J. Nick Koston <nick@koston.org>
Looks like there is a test failure otherwise this looks good I'll factory reset mine this weekend and try it out when I can afford to have it offline |
Thanks for your time! I have fixed the tests locally, CI should run fine now |
Dismissing stale review as it appears all comments have been addressed
Factory reset was a bit of a PITA but it works great!
|
Thanks @boswelja |
Agree. I assume this merge is just the first step towards parity with the third-party repo https://github.com/archerne/hastarlink https://community.home-assistant.io/t/starlink-dish-and-network-stats/297155 |
Right! Getting things merged into mainline is hard enough, not complaining ;) Great work getting this merged! |
That's what I had it at, but we're not supposed to be modifying data from the Py library/API
Guidelines say we can only add one platform at a time. You are showing binary sensors and buttons. I have a PR up for binary sensors now: #85409 |
Is scaling/unit conversion really modifying? |
Considering I was asked to stop doing that, yes Edit: You're welcome to read the PR review comments, it's all covered there |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the comments in a new PR. Thanks!
icon="mdi:clock", | ||
device_class=SensorDeviceClass.TIMESTAMP, | ||
entity_category=EntityCategory.DIAGNOSTIC, | ||
value_fn=lambda data: datetime.now().astimezone() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use our dt utils as far as possible.
Lines 108 to 109 in ccd8bc1
def now(time_zone: dt.tzinfo | None = None) -> dt.datetime: | |
"""Get now in specified time zone.""" |
device_class=SensorDeviceClass.TIMESTAMP, | ||
entity_category=EntityCategory.DIAGNOSTIC, | ||
value_fn=lambda data: datetime.now().astimezone() | ||
- timedelta(seconds=data["uptime"]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this calculation stable so that the timestamp state doesn't update spammily?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't noticed any issues, but I also haven't run this for longer than maybe a minute at a time. I can leave it polling for a while and see if you suspect there might be an issue?
Proposed change
Add support for loading data from a Starlink terminal. I have tested the integration with my square Dishy & new terminal, and the underlying python library
starlink-grpc-core
has been used for both (new) square Dishy and (old) round Dishy.Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: