-
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
Se05x HSM support in thermostat example #22220
Se05x HSM support in thermostat example #22220
Conversation
PR #22220: Size comparison from 55f9049 to bd6375b Increases (6 builds for bl602, cc13x2_26x2, nrfconnect, telink)
Decreases (6 builds for cc13x2_26x2, cyw30739, psoc6)
Full report (30 builds for bl602, cc13x2_26x2, cyw30739, efr32, k32w, linux, mbed, nrfconnect, psoc6, telink)
|
PR #22220: Size comparison from 16edce7 to 1592ed1 Increases (2 builds for bl602, cc13x2_26x2)
Decreases (7 builds for cc13x2_26x2, cyw30739, k32w, nrfconnect, psoc6, telink)
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, telink)
|
PR #22220: Size comparison from 16edce7 to eacc3fb Increases (2 builds for bl602, cc13x2_26x2)
Decreases (7 builds for bl602, cc13x2_26x2, k32w, nrfconnect, psoc6, telink)
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, telink)
|
examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp
Outdated
Show resolved
Hide resolved
examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp
Outdated
Show resolved
Hide resolved
examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp
Outdated
Show resolved
Hide resolved
examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp
Outdated
Show resolved
Hide resolved
examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp
Outdated
Show resolved
Hide resolved
examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp
Outdated
Show resolved
Hide resolved
examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp
Outdated
Show resolved
Hide resolved
examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp
Outdated
Show resolved
Hide resolved
examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp
Outdated
Show resolved
Hide resolved
examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp
Outdated
Show resolved
Hide resolved
PR #22220: Size comparison from 8af13a2 to d35dbf0 Increases (3 builds for nrfconnect, psoc6, telink)
Decreases (7 builds for bl602, psoc6, qpg, telink)
Full report (33 builds for bl602, cc13x2_26x2, cyw30739, efr32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
PR #22220: Size comparison from 989ad8e to cf45b00 Increases (9 builds for bl602, bl702, esp32, psoc6, telink)
Decreases (3 builds for bl702, esp32, nrfconnect)
Full report (37 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
CHIP_ERROR_INTERNAL); | ||
} | ||
/* Set time stamp length */ | ||
VerifyOrReturnError(CHIP_NO_ERROR == se05xSetCertificate(CD_TIME_STAMP_LEN_SE05X_ID, &tslen, 1), CHIP_ERROR_INTERNAL); |
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.
Why isn't this using time_stamp.size()
? That is, what is the point of tslen
at all? It seems like an attractive nuisance, as do most of the GetLength
calls here.
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.
get() member will return error in case the tag length is 0. So using the GetLength() before get() member is called.
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.
Wait, what? Why would Get
return an error on 0 length? It most certainly does not do that...
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 have created the issue to check this as well as other changes you suggested in the previous conversation. (#23064). Will address these in the next PR.
/* Get nocsr */ | ||
ReturnErrorOnFailure(tagReader.Get(csr_data)); | ||
/* Set nocsr length */ | ||
VerifyOrReturnError(CHIP_NO_ERROR == se05xSetCertificate(NOCSR_CSR_LEN_SE05X_ID, &csrlen, 1), CHIP_ERROR_INTERNAL); |
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.
Why not initializing csrlen from csr_data.size()
?
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.
get() member will return error in case the tag length is 0. So using the GetLength() before get() member is called.
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.
Again, Get
does not return error on zero length....
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 have created the issue to check this as well as other changes you suggested in the previous conversation. (#23064). Will address these in the next PR.
PR #22220: Size comparison from 33b4fab to 9d1a84a Increases (9 builds for bl602, bl702, cc13x2_26x2, psoc6, telink)
Decreases (5 builds for bl702, cc13x2_26x2, nrfconnect, psoc6)
Full report (30 builds for bl602, bl702, cc13x2_26x2, cyw30739, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
PR #22220: Size comparison from a334714 to 9c053fa Increases (5 builds for bl602, bl702, psoc6, telink)
Decreases (4 builds for bl602, bl702, esp32, telink)
Full report (37 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
PR #22220: Size comparison from 35703fc to be4770c Increases (6 builds for bl602, cc13x2_26x2, psoc6, telink)
Decreases (6 builds for cc13x2_26x2, nrfconnect, psoc6)
Full report (35 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, mbed, nrfconnect, psoc6, qpg, telink)
|
PR #22220: Size comparison from b6c9191 to ac37f45 Increases (6 builds for cc13x2_26x2, esp32, psoc6)
Decreases (7 builds for bl602, bl702, cc13x2_26x2, psoc6, telink)
Full report (37 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
PR #22220: Size comparison from fa81768 to 0bd1383 Increases (6 builds for bl602, bl702, psoc6, telink)
Decreases (3 builds for esp32, telink)
Full report (37 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
Problem
Se05x HSM support in thermostat example
Change overview
Testing
Tested using the thermostat example and chip tool for connectivity