modules: dataman: Optimize memory usage #6809
Conversation
|
I have not fully reviewed this - but what happens if you already have a mission stored and its re-loaded after this diff is applied? Did you guard against that? |
|
The DM_KEY_COMPAT will not match and it will erase all missions. |
|
As long as you have tested that it deletes the mission that's fine. |
Tested with file backend over STIL and RAM_FLASH over AeroFC. |
zehortigoza
force-pushed
the
dataman-size
branch
3 times, most recently
from
4b799cb to
4288a7f
Compare
|
ping? |
src/systemcmds/tests/test_dataman.c
Outdated
| struct dataman_compat_s compat; | ||
| } dataman_max_size_t; | ||
|
|
||
| #define DM_MAX_DATA_SIZE sizeof(dataman_max_size_t) |
There was a problem hiding this comment.
I think we can simplify this to #define DM_MAX_DATA_SIZE sizeof(struct mission_s) so we don't need the struct.
Also, not related, but I noticed the return of task_main is not checked, thus the unit test can fail but the result still shows as success.
There was a problem hiding this comment.
Well I just moved it I think is good keep this way in case the structs change size and other struct becomes bigger than struct mission_s also the compiler will optimize it anyway.
There was a problem hiding this comment.
Sure but we don't really need the maximum size here. My reasoning is that wenn adding new dataman item types or exchange structs, this place will likely be forgotten to be updated. So by simplifying we make it less prone to mistakes.
Also since we change the internal semantics, there is no global maximum item size anymore, but the size depends on the item type. The tests should take that into account.
There was a problem hiding this comment.
Well this buffers is used to read more than one type of dm_item_t but if you think is better I just pushed a version changing to struct mission_s and also another patch that check for the result of each task.
Use the size of each item type instead of the biggest one. In AeroFC that runs is constrained mode it was using 7860 bytes and now it uses 6930 bytes almost 1KB less.
…need to persist Just update in RAM is enough.
zehortigoza
force-pushed
the
dataman-size
branch
from
4288a7f to
5dbf45f
Compare
|
ping2? |
|
We plan to merge this after the next release. |
|
ping? |
|
Release is out, time to merge this! |
|
Thanks =D |
Use the size of each item type instead of the biggest one.
In AeroFC that runs is constrained mode it was using 7860 bytes
and now it uses 6930 bytes almost 1KB less.