search

nasa / fprime

F´ - A flight software and embedded systems framework
https://fprime.jpl.nasa.gov
Apache License 2.0
10.08k stars 1.31k forks source link

Update `ActiveTextLogger` naming to match build system naming expectations #2509

Closed kbotteon closed 9 months ago

kbotteon commented 9 months ago
Related Issue(s)
Has Unit Tests (y/n) Yes
Documentation Included (y/n) Yes

Change Description

This PR updates file names and CMakeLists for ActiveTextLogger to conform with what I believe the build system requires. See Rationale below.

I could also be completely overlooking a pattern or small detail, so please let me know if this change is unnecessary! I can't imagine I'm the first person to run into this problem with such a common Component, so I suspect another issue in my configuration might be involved.

Rationale

Including ActiveTextLogger in a topology leads to a compiler error in an auto-coded file:

TopologyAc.hpp:16:10: fatal error: Svc/ActiveTextLogger/ActiveTextLogger.hpp: No such file or directory
 #include "Svc/ActiveTextLogger/ActiveTextLogger.hpp"

If I change the fpp to use ActiveTextLoggerImpl, now the build fails at fpp-to-cpp:

Declarations.fpp:79.23
instance logText: Svc.ActiveTextLoggerImpl \
                      ^
error: undefined symbol ActiveTextLoggerImpl

I think the latest build system expects naming like Svc/<ComponentName>/<ComponentName> without the ComponentImpl class or Impl file suffixes.

Testing/Review Recommendations

My topology including ActiveTextLogger now compiles. fprime-util check –all runs with 100% tests passed, 0 tests failed out of 54.

LeStarch commented 9 months ago

@kbotteon we previously added headers that typedefed the Impl version of the class to the non-impl version. These headers were named with the new preferred name.

However, renaming is a much better fix and since you are doing a single component....I wholeheartedly approve!