Skip to content

Do not crash on missing build number in fw version #1500

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Feb 10, 2025
Merged

Do not crash on missing build number in fw version #1500

merged 5 commits into from
Feb 10, 2025

Conversation

rytilahti
Copy link
Member

Avoid parsing firmware version if it doesn't contain a whitespace (i.e., build number).

Fixes #1499

@rytilahti rytilahti added the bug Something isn't working label Feb 9, 2025
@rytilahti rytilahti mentioned this pull request Feb 9, 2025
Closed
@codecov Codecov
Copy link

codecov bot commented Feb 9, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 53.84615% with 6 lines in your changes missing coverage. Please review.

Project coverage is 92.66%. Comparing base (d5187dc) to head (cf165fc).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
kasa/iot/iotdevice.py 33.33% 1 Missing and 1 partial ⚠️
kasa/smartcam/smartcamchild.py 33.33% 1 Missing and 1 partial ⚠️
kasa/smartcam/smartcamdevice.py 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1500      +/-   ##
==========================================
- Coverage   92.71%   92.66%   -0.06%     
==========================================
  Files         150      150              
  Lines        9530     9538       +8     
  Branches      970      974       +4     
==========================================
+ Hits         8836     8838       +2     
- Misses        496      499       +3     
- Partials      198      201       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sdb9696
sdb9696 approved these changes Feb 9, 2025
View reviewed changes
Copy link
Collaborator

@sdb9696 sdb9696 left a comment

Choose a reason for hiding this comment

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

LGTM

@sdb9696
Copy link
Collaborator

sdb9696 commented Feb 10, 2025 •
edited
Loading

I've updated this PR to treat the fixture as a modify to the existing 1.5.0, as I've confirmed it's exactly the same data (other than the fw_ver), and we don't currently treat the build number as a filename key (avoids two identical lines in SUPPORTED.md). Also fixed dump_devinfo as the get_temp_humidity_records call was not being made as part of the smartcam hub fixture generation.

EDIT: Also applied the same check for no build number to smartcam and iot devices.

@sdb9696 sdb9696 added this to the 0.10.2 milestone Feb 10, 2025
@sdb9696 sdb9696 enabled auto-merge (squash) February 10, 2025 11:11
@sdb9696 sdb9696 merged commit 668e32d into master Feb 10, 2025
16 of 18 checks passed
@sdb9696 sdb9696 deleted the fix/missing_rel_in_fwver branch February 10, 2025 11:13
@rytilahti
Copy link
Member Author

I was wondering if it'd be better to try-except on version parsing, but this works fine too. Thanks for fixing the CI & merging it! :-)

@sdb9696 sdb9696 mentioned this pull request Feb 12, 2025
Merged
sdb9696 added a commit that referenced this pull request Feb 12, 2025
@sdb9696
Prepare 0.10.2 (#1505)
f488492 
## [0.10.2](https://github.com/python-kasa/python-kasa/tree/0.10.2) (2025-02-12)

[Full Changelog](0.10.1...0.10.2)

**Release summary:**

- Bugfix for [#1499](#1499).
- Support for L530B and C110 devices.

**Fixed bugs:**

- H100 - Raised error: not enough values to unpack \(expected 2, got 1\) [\#1499](#1499)
- Do not crash on missing build number in fw version [\#1500](#1500) (@rytilahti)

**Added support for devices:**

- Add C110\(EU\) 2.0 1.4.3 fixture [\#1503](#1503) (@sdb9696)
- Add L530B\(EU\) 3.0 1.1.9 fixture [\#1502](#1502) (@sdb9696)

**Project maintenance:**

- Add fixtures for new versions of H100, P110, and T100 devices [\#1501](#1501) (@LXGaming)
- Add L530E\(TW\) 2.0 1.1.1 fixture [\#1497](#1497) (@bluehomewu)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdb9696 sdb9696 sdb9696 approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
0.10.2
Development

Successfully merging this pull request may close these issues.

H100 - Raised error: not enough values to unpack (expected 2, got 1)
2 participants
@rytilahti @sdb9696