Add Fw::ObjectName to hold Fw::ObjBase name #2497

thomas-bc · 2024-01-23T02:53:51Z


*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: nasa/fpp#385

Rationale

See #2493

Fw/Types/ObjectName.hpp

LeStarch

A few very minor style guidelines.

Fw/Types/ObjectName.hpp

bocchino · 2024-01-23T22:13:12Z

This looks great! I suggest we do the following:

Merge Data product framework support #2485. This will bring fpp/main into sync with fprime/devel.
Merge Add support for Fw::ObjectName fpp#385.
Make a new pre-release of FPP.
Update this PR to refer to the FPP pre-release and pass CI.
Merge this PR.

thomas-bc · 2024-01-24T01:52:02Z

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.

Don't include ObjectName header unless object names are enabled

bocchino · 2024-02-26T18:06:17Z

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.

thomas-bc · 2024-02-26T22:27:51Z

@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?

thomas-bc · 2024-02-26T22:31:28Z

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

bocchino · 2024-02-26T22:41:42Z

I agree, let's defer this to a separate issue.

Fw/Obj/ObjBase.cpp

Fw/Types/ObjectName.cpp

Fw/Comp/PassiveComponentBase.cpp

Fw/Comp/ActiveComponentBase.cpp

Fw/Comp/QueuedComponentBase.cpp

Fw/Obj/ObjBase.cpp

Fw/Types/ObjectName.cpp

Fw/Types/ObjectName.hpp

Fw/Types/ObjectName.cpp

LeStarch

I removed the CI warnings that we shouldn't fix. You should address the rest.

With the "side effect in boolean" we should fix that by assigning to a const variable and then checking.

The others I think are mostly checking return of string_copy. It should check and null terminate if possible.

Finally nullptr checks on CHAR* pointers

Fw/Comp/ActiveComponentBase.cpp

Fw/Comp/PassiveComponentBase.cpp

Fw/Comp/QueuedComponentBase.cpp

Fw/Obj/ObjBase.cpp

Fw/Port/InputPortBase.cpp

Fw/Types/ObjectName.cpp

Fw/Comp/QueuedComponentBase.cpp

Fw/Comp/ActiveComponentBase.cpp

Fw/Obj/ObjBase.cpp

Fw/Port/InputPortBase.cpp

Fw/Comp/PassiveComponentBase.cpp

@@ -13,7 +13,8 @@
    void PassiveComponentBase::toString(char* buffer, NATIVE_INT_TYPE size) {
        FW_ASSERT(buffer);
        FW_ASSERT(size > 0);
-        if (snprintf(buffer, size, "Comp: %s", this->m_objName) < 0) {
+        PlatformIntType status = snprintf(buffer, size, "Comp: %s", this->m_objName.toChar());


Fw/Comp/QueuedComponentBase.cpp

@@ -22,7 +22,8 @@
 #if FW_OBJECT_TO_STRING == 1 && FW_OBJECT_NAMES == 1
    void QueuedComponentBase::toString(char* buffer, NATIVE_INT_TYPE size) {
        FW_ASSERT(size > 0);
-        if (snprintf(buffer, size,"QueueComp: %s", this->m_objName) < 0) {
+        PlatformIntType status = snprintf(buffer, size, "QueueComp: %s", this->m_objName.toChar());


Fw/Comp/ActiveComponentBase.cpp

@@ -45,7 +45,8 @@
 #if FW_OBJECT_TO_STRING == 1 && FW_OBJECT_NAMES == 1
    void ActiveComponentBase::toString(char* buffer, NATIVE_INT_TYPE size) {
        FW_ASSERT(size > 0);
-        if (snprintf(buffer, size, "ActComp: %s", this->m_objName) < 0) {
+        PlatformIntType status = snprintf(buffer, size, "ActComp: %s", this->m_objName.toChar());


Fw/Port/InputPortBase.cpp

@@ -29,8 +29,9 @@
    void InputPortBase::toString(char* buffer, NATIVE_INT_TYPE size) {
 #if FW_OBJECT_NAMES == 1 
        FW_ASSERT(size > 0);
-        if (snprintf(buffer, size, "InputPort: %s->%s", this->m_objName,
-                     this->isConnected() ? this->m_connObj->getObjName() : "None") < 0) {
+        PlatformIntType status = snprintf(buffer, size, "InputPort: %s->%s", this->m_objName.toChar(),


Fw/Obj/ObjBase.cpp

    }
 #if FW_OBJECT_TO_STRING == 1
    void ObjBase::toString(char* str, NATIVE_INT_TYPE size) {
        FW_ASSERT(size > 0);
-        if (snprintf(str, size, "Obj: %s",this->m_objName) < 0) {
+        PlatformIntType status = snprintf(str, size, "Obj: %s", this->m_objName.toChar());


Fw/Types/ObjectName.cpp

thomas-bc added 3 commits January 22, 2024 12:53

Implement Fw::ObjectName

345e3ec

format

abc9cdf

Add test case

1b67fd9

thomas-bc mentioned this pull request Jan 23, 2024

Add support for Fw::ObjectName nasa/fpp#385

Merged

github-advanced-security bot found potential problems Jan 23, 2024

View reviewed changes

Fw/Types/ObjectName.hpp Fixed Show resolved Hide resolved

Fw/Types/ObjectName.hpp Fixed Show resolved Hide resolved

Fix PolyDb UT and spelling

42332a5

LeStarch requested changes Jan 23, 2024

View reviewed changes

Fw/Types/ObjectName.hpp Outdated Show resolved Hide resolved

Fw/Types/ObjectName.hpp Fixed Show resolved Hide resolved

Fw/Types/ObjectName.hpp Fixed Show resolved Hide resolved

Fw/Types/ObjectName.hpp Outdated Show resolved Hide resolved

Explicit constructor and use CHAR

384d1d2

bocchino added 2 commits February 26, 2024 08:56

Merge remote-tracking branch 'upstream/devel' into Fw-ObjectName

1af23ce

Revise ObjBase.hpp

02c9cb9

Don't include ObjectName header unless object names are enabled

thomas-bc mentioned this pull request Feb 26, 2024

Rename *_MAX_SIZE constants in FpConfig.h #2545

Closed

Update requirements.txt for fpp==2.1.0a4

387b969

github-advanced-security bot found potential problems Feb 27, 2024

View reviewed changes

Fw/Types/ObjectName.cpp Dismissed Show dismissed Hide dismissed

Fw/Types/ObjectName.hpp Dismissed Show dismissed Hide dismissed

Fw/Types/ObjectName.cpp Dismissed Show dismissed Hide dismissed

Fw/Types/ObjectName.cpp Fixed Show fixed Hide fixed

Fw/Types/ObjectName.cpp Fixed Show fixed Hide fixed

bocchino requested a review from LeStarch February 27, 2024 01:27

LeStarch requested changes Feb 28, 2024

View reviewed changes

Fix static code analysis warnings

75f3c04

github-advanced-security bot found potential problems Mar 1, 2024

View reviewed changes

Fw/Comp/QueuedComponentBase.cpp Fixed Show fixed Hide fixed

Fw/Comp/ActiveComponentBase.cpp Fixed Show fixed Hide fixed

Fw/Obj/ObjBase.cpp Fixed Show fixed Hide fixed

Fw/Port/InputPortBase.cpp Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Mar 1, 2024

View reviewed changes

cast string_copy return value to void

ce61d97

github-advanced-security bot found potential problems Mar 1, 2024

View reviewed changes

Fw/Types/ObjectName.cpp Dismissed Show resolved Hide resolved

Fw/Types/ObjectName.cpp Dismissed Show dismissed Hide dismissed

Fw/Types/ObjectName.cpp Dismissed Show dismissed Hide dismissed

Fw/Types/ObjectName.cpp Dismissed Show dismissed Hide dismissed

github-advanced-security bot found potential problems Mar 1, 2024

View reviewed changes

Fw/Types/ObjectName.cpp Dismissed Show dismissed Hide dismissed

Fw/Types/ObjectName.cpp Dismissed Show dismissed Hide dismissed

thomas-bc mentioned this pull request Mar 1, 2024

Resolve string_copy static analysis warnings #2556

Merged

Resolve unchecked argument

4d245d7

LeStarch approved these changes Mar 5, 2024

View reviewed changes

LeStarch merged commit bdfe241 into nasa:devel Mar 5, 2024
44 checks passed

thomas-bc added the Update Instructions Needed Need to add instructions in the release notes for updates. label Mar 13, 2024

thomas-bc deleted the Fw-ObjectName branch June 25, 2024 18:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Fw::ObjectName to hold Fw::ObjBase name #2497

Add Fw::ObjectName to hold Fw::ObjBase name #2497

thomas-bc commented Jan 23, 2024

LeStarch left a comment

bocchino commented Jan 23, 2024 •

edited by thomas-bc

Loading

thomas-bc commented Jan 24, 2024

bocchino commented Feb 26, 2024

thomas-bc commented Feb 26, 2024 •

edited

Loading

thomas-bc commented Feb 26, 2024

bocchino commented Feb 26, 2024

LeStarch left a comment

Add Fw::ObjectName to hold Fw::ObjBase name #2497

Add Fw::ObjectName to hold Fw::ObjBase name #2497

Conversation

thomas-bc commented Jan 23, 2024

Change Description

Rationale

LeStarch left a comment

Choose a reason for hiding this comment

bocchino commented Jan 23, 2024 • edited by thomas-bc Loading

thomas-bc commented Jan 24, 2024

bocchino commented Feb 26, 2024

thomas-bc commented Feb 26, 2024 • edited Loading

thomas-bc commented Feb 26, 2024

bocchino commented Feb 26, 2024

LeStarch left a comment

Choose a reason for hiding this comment

bocchino commented Jan 23, 2024 •

edited by thomas-bc

Loading

thomas-bc commented Feb 26, 2024 •

edited

Loading