Skip to content
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

Module list is duplicated/hard coded so can't be modified #1287

Closed
skliper opened this issue Apr 8, 2021 · 7 comments · Fixed by #1290 or #1406
Closed

Module list is duplicated/hard coded so can't be modified #1287

skliper opened this issue Apr 8, 2021 · 7 comments · Fixed by #1290 or #1406
Assignees
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Apr 8, 2021

Is your feature request related to a problem? Please describe.
Part of the intent of modules was to be able to replace them. Hard-coding of the module list at:

set(core_api_MODULES es evs fs msg resourceid sb tbl time osal psp)

defeats this capability.

Describe the solution you'd like
Define the module list in a way it can be modified/customized by configuration.

Describe alternatives you've considered
None

Additional context
Currently breaks message header customization.

Requester Info
Jacob Hageman - NASA/GSFC

Ping - @excaliburtb

@skliper skliper added this to the 7.0.0 milestone Apr 8, 2021
@jphickey
Copy link
Contributor

jphickey commented Apr 8, 2021

That's not what this is - this is just for the API. One can still customize the implementation of any module.

The API needs to have some standards/baselines - while the implementation can be overridden, it needs to conform to the same API. The list in this file is what defines the notion of "cfe core API" as a whole.

I can see where this might not work if the name was changed, though. Is the intent that one not only wants to override the module implementation, but also change its name i.e. to have the "tbl" functions provided by a module not called "tbl"?

@skliper
Copy link
Contributor Author

skliper commented Apr 8, 2021

It's pulling in the include directories and compile definitions of the hard coded modules... that's broken.

@jphickey
Copy link
Contributor

jphickey commented Apr 8, 2021

Not necessarily the ones in modules/ though - if there is a user-provided implementation, that will be used instead, and it should be the interface definitions from the user-provided implementation, not the default one.

The assumption here is that it (i.e. the user provided module) will have the same target name, is that not correct?

@skliper
Copy link
Contributor Author

skliper commented Apr 8, 2021

Correct, projects should be able to use whatever names they want. The previously working example (see #879).

@skliper
Copy link
Contributor Author

skliper commented Apr 8, 2021

They should be able to remove/add/replace modules... really just need to supply implementations of all the APIs.

@jphickey
Copy link
Contributor

jphickey commented Apr 8, 2021

They can remove/add/replace modules, that all works. The problem is that we need to know whether that module is intended to provide something that is conceptually part of the "core_api" or if it is some user/mission thing that is not part of the core api.

Case in port, for #879 - we don't know the intent of a module that is named custom_msg - that is, whether it is intended to provide the msg interface that is part of core_api or whether it is just some other app/lib that happens to be named custom_msg and does something else.

This (perhaps incorrectly) assumed that if one was overriding the msg lib, it would still be called msg, which is the only way to know if it is intended to fulfill that role.

Would an alias target be OK? That might work...

@jphickey
Copy link
Contributor

jphickey commented Apr 8, 2021

To clarify that "alias" thought a bit - what I'd suggest is that in the CMakeLists.txt for the custom_msg module, it does this:

# Indicate that this is intended to fulfill the role of "msg" core interface
add_library(msg ALIAS custom_msg)

jphickey added a commit to jphickey/cFE that referenced this issue Apr 9, 2021
Separate the list of CFE core interface modules (e.g. core_api) from
the list of CFE core implementation modules (e.g. msg).  This allows
the content of core_api to be expanded to include any additional
modules the user has added to cFE core locally.
astrogeco added a commit that referenced this issue Apr 22, 2021
Fix #1287, split interface and implementation modules
zanzaben pushed a commit to zanzaben/cFE that referenced this issue Apr 22, 2021
Fix nasa#1287, split interface and implementation modules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants