-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[controller] Fix flake8 warnings #25311
Conversation
PR #25311: Size comparison from 3b2f2d2 to 134362f Increases (9 builds for bl702, cc32xx, psoc6, telink)
Decreases (8 builds for bl602, esp32, nrfconnect, telink)
Full report (57 builds for bl602, bl702, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
134362f
to
d5b52b1
Compare
PR #25311: Size comparison from 2dd4fdb to d5b52b1 Increases (6 builds for qpg, telink)
Decreases (17 builds for bl602, bl702, bl702l, cc32xx, nrfconnect, psoc6, qpg, telink)
Full report (58 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
PR #25311: Size comparison from 2dd4fdb to 510a445 Increases (10 builds for bl702, esp32, psoc6, qpg, telink)
Decreases (8 builds for esp32, psoc6, telink)
Full report (58 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
510a445
to
2fb0fd7
Compare
PR #25311: Size comparison from 23a0064 to 2fb0fd7 Increases (13 builds for bl702, bl702l, psoc6, telink)
Decreases (8 builds for esp32, nrfconnect, psoc6, telink)
Full report (58 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
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.
Generally OK, but please consider some fixes for my comments.
Restyled by isort Restyled
2fb0fd7
to
104d6f2
Compare
PR #25311: Size comparison from 8291037 to 104d6f2 Increases (1 build for qpg)
Decreases (3 builds for bl602, bl702, cc32xx)
Full report (14 builds for bl602, bl702, bl702l, cc32xx, linux, mbed, nrfconnect, qpg)
|
104d6f2
to
f9d0bce
Compare
PR #25311: Size comparison from 8291037 to f9d0bce Increases (15 builds for bl602, bl702l, esp32, psoc6, qpg, telink)
Decreases (8 builds for cc32xx, nrfconnect, psoc6, telink)
Full report (58 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
PR #25311: Size comparison from 7ae5919 to e1b6830 Increases (8 builds for bl702, esp32, psoc6, qpg, telink)
Decreases (14 builds for bl602, cc32xx, psoc6, telink)
Full report (58 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
Commit 79cebcf ("[controller] Fix flake8 warnings (project-chip#25311)") changed the exceptions catched from a global catch to a specific catch for chip.exceptions.ChipStackException exceptions. However, in this two cases the error is just converted to a Python enum, there is no case where chip.exceptions.ChipStackException would be raised. Since those callbacks are called from the C++ SDK, to the best of my knowledge, the intention was that exception would not bubble up into SDK code. Handle the Python specific exceptions and make sure they get logged verbosley.
* Improve/fix exception handling in attribute update callbacks Commit 79cebcf ("[controller] Fix flake8 warnings (#25311)") changed the exceptions catched from a global catch to a specific catch for chip.exceptions.ChipStackException exceptions. However, in this two cases the error is just converted to a Python enum, there is no case where chip.exceptions.ChipStackException would be raised. Since those callbacks are called from the C++ SDK, to the best of my knowledge, the intention was that exception would not bubble up into SDK code. Handle the Python specific exceptions and make sure they get logged verbosley. * [Python] Simplify UpdateCachedData for better readability Extract cache type implementation in functions. Also make the code a bit more Pythonic. Note: This no longer initializes endpointCache in the Cluster-View. This shouldn't matter in practice as the dictionary wasn't used.
This pull request is a part of #25193 ( part fix python files in controller )
Problem
Python files need prevent things like syntax errors, typos, bad style, etc... it saves time for reviewing your code. Many python files needed bug fixes.
Changes
Fix all python files in controller where linter find problem. Adding controller to check with flake8.
Testing
CI will test and maybe some another manual testing