-
Notifications
You must be signed in to change notification settings - Fork 5
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
Updated code for API 72.1 #6
base: master
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request updates the File-Level Changes
Tips
|
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.
Hey @lucaspwo - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a brief comment explaining the reason for this API change and its implications.
- The furi_assert for event_queue was removed. Consider adding it back after extracting event_queue from ctx to maintain parameter validation.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
static void input_callback(InputEvent* input_event, void* ctx) { | ||
|
||
FuriMessageQueue* event_queue = ctx; |
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.
suggestion: Consider adding type safety checks for the ctx parameter
The change from FuriMessageQueue* to void* removes type safety at the function interface. Consider adding a static_assert or similar compile-time check to ensure ctx is of the correct type. Also, the removal of furi_assert(event_queue) eliminates an explicit null check. Consider re-adding this or a similar check to fail early if preconditions aren't met.
static void input_callback(InputEvent* input_event, void* ctx) { | |
FuriMessageQueue* event_queue = ctx; | |
static void input_callback(InputEvent* input_event, void* ctx) { | |
FuriMessageQueue* event_queue = ctx; | |
furi_assert(event_queue); | |
_Static_assert(sizeof(FuriMessageQueue*) == sizeof(void*), "Incompatible pointer sizes"); |
Modified the
input_callback
function to conform with API 72.1.Summary by Sourcery
Update the
input_callback
function to conform with API 72.1 by changing the parameter type for the event queue to a context pointer.Enhancements:
input_callback
function to use a context pointer for the event queue, aligning with API 72.1 requirements.