Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix blinkapp session call #782

Closed
wants to merge 5 commits into from
Closed

Fix blinkapp session call #782

wants to merge 5 commits into from

Conversation

mkmer
Copy link
Contributor

@mkmer mkmer commented Oct 11, 2023

Description:

Fix the auth call to use existing session

Related issue (if applicable): fixes #759

Checklist:

  • Local tests with tox run successfully PR cannot be meged unless tests pass
  • Changes tested locally to ensure platform still works as intended
  • Tests added to verify new code works

@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #782 (c078cb8) into dev (0c7c3a1) will increase coverage by 0.01%.
Report is 39 commits behind head on dev.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #782      +/-   ##
==========================================
+ Coverage   99.64%   99.65%   +0.01%     
==========================================
  Files           8        8              
  Lines        1404     1445      +41     
==========================================
+ Hits         1399     1440      +41     
  Misses          5        5              
Flag Coverage Δ
unittests 99.65% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
blinkpy/api.py 100.00% <100.00%> (ø)
blinkpy/camera.py 98.43% <100.00%> (+0.05%) ⬆️
blinkpy/sync_module.py 100.00% <100.00%> (ø)

Copy link
Owner

Choose a reason for hiding this comment

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

I think this was accidentally committed to the repo

network_id = json_data.get("network_id")
command_id = json_data.get("id")
if command_id and network_id:
while True:
Copy link
Owner

Choose a reason for hiding this comment

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

My aversion to while True loops remains. Since we're doing a sleep command here, can we change this to:

for i in range(0, SOME_MAX_TIMEOUT):

since in this case we know the SOME_MAX_TIMEOUT variable would map to a certain number of polls/certain amount of time.

@fronzbot
Copy link
Owner

See #772 I think my comments apply there, not here. I believe that one is supposed to be merged first and just fell off my radar, my bad.

@mkmer
Copy link
Contributor Author

mkmer commented Oct 11, 2023

I messed up, this was only supposed to have the session = session. I'll close this and do a new one with the intended fix.

@mkmer mkmer closed this Oct 11, 2023
@mkmer mkmer deleted the Fix-blinkapp branch October 11, 2023 14:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem after update...
2 participants