You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors the project to support bare-metal targets by separating core logic from platform-specific code and introducing HAL bridges for UART and sockets. Feedback focuses on optimizing performance by caching socket options, preventing CPU starvation in FreeRTOS tasks, and improving code safety and style through defensive pointer checks and consistent use of NULL.
The reason will be displayed to describe this comment to others. Learn more.
The task enters a busy-wait loop if the agent loop exits. In a FreeRTOS environment, this will starve lower-priority tasks from executing if this task has a higher priority. It is recommended to delete the task or use a blocking delay to yield the CPU.
The reason will be displayed to describe this comment to others. Learn more.
Calling setsockopt on every recv is inefficient as it involves a system call. Since the timeout is often constant for a session, consider caching the current timeout value in the ec_socket structure and only invoking setsockopt if the requested timeout_ms has changed.
The reason will be displayed to describe this comment to others. Learn more.
Since this bare-metal port uses a static singleton s_socket, it is safer to verify that the provided sock pointer actually matches the address of the singleton. This provides a defensive guard against callers passing invalid or uninitialized pointers.
Suggested change
if (!sock|| !sock->in_use|| !s_hal|| !s_hal->send) {
if (sock!=&s_socket|| !sock->in_use|| !s_hal|| !s_hal->send) {
This file contains hidden or 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
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.
Summary
Verification