-
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
[EFR32] Add support for TestEventTrigger on Silabs #27471
[EFR32] Add support for TestEventTrigger on Silabs #27471
Conversation
PR #27471: Size comparison from 7806dd1 to ce9d5a7 Increases (7 builds for esp32, psoc6, qpg, telink)
Decreases (15 builds for bl602, bl702, cc32xx, esp32, psoc6, telink)
Full report (46 builds for bl602, bl702, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
return -1; | ||
} | ||
|
||
static size_t hex_string_to_binary(const char * hex_string, uint8_t * buf, size_t buf_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.
This is reinventing code the SDK already has, for what it's worth @ericzijian1994 . That's bad in an example that you expect applications to follow...
* @param eventTrigger Event trigger to handle. | ||
* @return CHIP_NO_ERROR on success or another CHIP_ERROR on failure. | ||
*/ | ||
CHIP_ERROR HandleEventTrigger(uint64_t eventTrigger) override; |
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.
This override is not actually implemented. How is this supposed to work?
return -1; | ||
} | ||
|
||
static size_t hex_string_to_binary(const char * hex_string, uint8_t * buf, size_t buf_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.
Again, this is code that already exists in the SDK.
Fixes #27455