fprime
fprime copied to clipboard
Add Fw::ObjectName to hold Fw::ObjBase name
Related Issue(s) | #2493 |
Has Unit Tests (y/n) | y |
Documentation Included (y/n) | n |
Change Description
Implements Fw::ObjectName
to hold names of ObjBase
objects instead of raw char arrays.
I tried to keep the changes to a minimum, so I'm using .toChar()
in most places.
Let me know if there's other patterns you think I should add here, or any other desired changes.
Associated PR to FPP: https://github.com/fprime-community/fpp/pull/385
Rationale
See #2493
This looks great! I suggest we do the following:
- [x] Merge https://github.com/nasa/fprime/pull/2485. This will bring fpp/main into sync with fprime/devel.
- [x] Merge https://github.com/fprime-community/fpp/pull/385.
- [x] Make a new pre-release of FPP.
- [x] Update this PR to refer to the FPP pre-release and pass CI.
- [ ] Merge this PR.
Added the recommended changes.
I wanted to apply them to the other Fw::*String
types to resolve the same warnings in those classes, but that broke some implicit conversions in, at least, some UTs (e.g. LoggerMain.cpp)
Didn't want to go out of scope for this PR or risk some downstream breakages so I didn't touch anything, but might be worth fixing in the future.
I am working on integrating this PR with FPP. It's looking good so far. Here is one issue I see: FW_OBJECT_NAME_MAX_SIZE
is technically not a correct name. It should be FW_OBJECT_NAME_BUFFER_SIZE
or something like that. The max string length is the buffer size minus one, because of the terminating zero. Compare FW_OBJ_TO_STRING_BUFFER_SIZE
, which gets this right.
@bocchino this also applies to FW_QUEUE_NAME_MAX_SIZE
, FW_TASK_NAME_MAX_SIZE
, and probably others. Do you want me to change these in this PR, or should we capture this in a new issue and get back to it in a different PR?
Since this is a breaking change for projects that define their own config/
folder, I would suggest we do the name changes all at once in a different PR, and keep the scope of this PR as is
I agree, let's defer this to a separate issue.