-
Notifications
You must be signed in to change notification settings - Fork 18
A new string-like data structure #247
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
base: master
Are you sure you want to change the base?
Conversation
LelsersLasers
left a comment
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 thought like 2% more about BMS since we last talked and I think a buffer type would not really be helpful as 99% of the time we will need to know which IC/BMS in the chain we are looking at/talking to and that will determine the index into a static buffer we write to instead of for i in bms_count: push(string).
Not that I don't think that the buffer type wouldn't be useful for other things though or even for easier writing of test code.
| } | ||
|
|
||
| typedef struct { | ||
| uint8_t *data; |
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.
Small: could be good to put a comment that data should live as the buffer
| buf->length = 0; | ||
| } | ||
|
|
||
| bool buffer_append(byte_buffer_t *buf, const void *data, size_t 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 think for most of what I could imagine us using a buffer type for right now we only care if we had a enough space or not, but I think it could be a good idea to slight future proof this and have it return size_t which is the number of bytes written from data into buf.
| size_t max_len; | ||
| } byte_buffer_t; | ||
|
|
||
| void buffer_clear(byte_buffer_t *buf); |
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.
Small: short comments? (Ex: clear: sets length to 0, does not affect contents of data?)
even if not used for BMS, Dashboard certainly needs it |
Useful for BMS and Dashboard LCD code