com: shrink send/receive buffers from 256 to 64 bytes #193
Conversation
WalkthroughThe changes mainly reduce the maximum message buffer length in the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Com
participant PacketSerial
App->>Com: Send Message
Com->>PacketSerial: Process Packet (Using MAX_MSG_BUFFER_LEN)
PacketSerial-->>Com: Acknowledge
Com-->>App: Message Sent Confirmation
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/ayab/com.h (2 hunks)
Additional comments not posted (2)
src/ayab/com.h (2)
139-139: Verify the compatibility and functionality of the newm_packetSerialtype.The type of
m_packetSerialhas been changed toPacketSerial_<SLIP, SLIP::END, MAX_MSG_BUFFER_LEN>. Ensure that this new type is compatible with existing code and provides the expected functionality.
47-47: Verify the adequacy of the new buffer size.The
MAX_MSG_BUFFER_LENhas been reduced from 255 to 64. Ensure that this new buffer size is sufficient for all use cases and does not lead to buffer overflows or truncated messages.
|
This should help quite a bit with #190 as well. |
Will do! |
|
@X-sam here's a manual PR to document the message size limit: AllYarnsAreBeautiful/ayab-manual#39 , please let me know what you think! |
The largest message that is generated by the current protocol is a `cnfLine` containing 25 bytes of pattern, plus 5 bytes of other data. Even assuming a worst case of having to SLIP-escape all the bytes, this fits in a 64-byte buffer.
Issue
As shown in #191 the memory usage of the firmware needs to be brought under control. A significant contributor to the allocated RAM is the set of message buffers used by the serial communication layer.
Proposed solution
The largest message that is generated by the current protocol is a
cnfLinecontaining 25 bytes of pattern, plus 5 bytes of other data. Even assuming a worst case of having to SLIP-escape all the bytes, this fits in a 64-byte buffer.This PR therefore reduces both the outgoing message buffer (
Com::msgBuffer) and the incoming message buffer in thePacketSerialinstance to 64 bytes each.If the objects are statically alllocated as proposed in #191 , it can be seen that the memory used goes from
91.3% (used 1869 bytes from 2048 bytes)to72.5% (used 1485 bytes from 2048 bytes)with this change.Summary by CodeRabbit
These changes aim to optimize communication within the application.