[Python] TLV schema tag should start from 1 #12347
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pullapprove
bot
requested review from
andy31415,
anush-apple,
austinh0,
balducci-apple,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chulspro,
Damian-Nordic,
electrocucaracha,
emargolis,
franck-apple,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs,
kghost,
LuDuda,
lzgrablic02,
mlepage-google,
msandstedt,
pan-apple,
robszewczyk,
sagar-apple,
saurabhst,
selissia and
tcarmelveilleux
pullapprove
bot
requested review from
tecimovic,
vivien-apple,
wbschiller,
woody-apple,
yufengwangca and
yunhanw-google
|
PR #12347: Size comparison from f50307a to 394c23b Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
woody-apple
approved these changes
Nov 30, 2021
mrjerryjohns
suggested changes
Nov 30, 2021
exceptions as needed in Matter modules to better debug issues that happen (like the one this PR is trying to solve). This is settable/clearable using the mattersetdebug() function. - Better format the exception object in the cluster object tests to return back useful information embedded in custom exception fields that can only be retrieved using str().
mrjerryjohns
approved these changes
Nov 30, 2021
There was a problem hiding this comment.
As per discussions with @erjiaqing, I have come around to the approach of not throwing exceptions up to the user, but rather, returning the result as ValueDecodeFailure objects that in turn, contain the exception. This permits retrieving all attribute data without terminally failing the whole interaction on a single bad attribute.
However, this does make debugging issues more difficult. I've gone ahead and added a mattersetdebug() function that has a global built-in enableDebugMode variable that can be used to raise exceptions instead when trying to debug these failures. That will in turn, return the exception traceback which is needed to debug stuff.
yufengwangca
approved these changes
Nov 30, 2021
mlepage-google
pushed a commit
to mlepage-google/connectedhomeip
that referenced
this pull request
Dec 1, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
TLV schema tag should start from 1, and the error is not captured.
Change overview
Update codegen, and add tests to verify if we successfully decoded all values.
Create
ValueDecodeFailureclass to wrap the value, since exceptions will be ignored in C callbacks.Wrap tests by _AssumeDecodeSuccess, users don't have to do this manually, they will see ValueDecodeFailure instead of the attribute describer.
Adds a new global debug mode variable that can be set using
mattersetdebug()to facilitate debugging of issues (e.g throwing exceptions in the relevant modules instead of returning formatted ValueDecodeFailure objects).Testing
CI with first commit only should fail: https://github.com/erjiaqing/connectedhomeip/actions/runs/1519033903
CI with both commits should success.