Skip to content

Commit

Permalink
Merge branch 'develop-remove-icarous' into develop. Close #237.
Browse files Browse the repository at this point in the history
**Description**

The cFS application template was originally built for ICAROUS, but it is now
generic to cFS. ICAROUS is still mentioned in the template, and the resulting
code will not compile unless ICAROUS is present as an app.

**Type**

- Bug: cFS template generates code that does not work in cFS, but requires a
  specific cFS application to be present.

**Additional context**

None.

**Requester**

- Ivan Perez.

**Method to check presence of bug**

Searching for ICAROUS in the template surfaces several occurrences:

```sh
$ grep -niHre 'icarous' ogma-core/templates/
ogma-core/templates/copilot-cfs/fsw/src/copilot_cfs.h:44:void COPILOT_ProcessIcarousPosition(void);
ogma-core/templates/copilot-cfs/fsw/src/copilot_cfs.c:19:#include "Icarous_msgids.h"
ogma-core/templates/copilot-cfs/fsw/src/copilot_cfs.c:20:#include "Icarous_msg.h"
ogma-core/templates/copilot-cfs/fsw/src/copilot_cfs.c:154:* Make ICAROUS data available to Copilot and run monitors.
ogma-core/templates/copilot-cfs/CMakeLists.txt:5:include_directories(../Icarouslib/fsw/platform_inc)
```

**Expected result**

Running the command above returns an empty output, showing that ICAROUS is not
mentioned in the template.

The following Dockerfile checks that generating a cFS application with and
without the feature proposed renders the same result, provided that the
additional data is given to the new cFS backend as an additional JSON file,
after which it prints the message "Success". There's a slight difference that
cannot be resolved via additional variables or flags, so that different is
disregarded by replacing the text with `sed` first. It requires an additional
`BASE_COMMIT` variable pointing to a commit prior to merging this feature.

```
FROM ubuntu:focal

ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get update

RUN apt-get install --yes \
      curl g++ gcc git libgmp3-dev libz-dev make pkg-config

RUN mkdir -p $HOME/.local/bin
ENV PATH=$PATH:/root/.local/bin/

RUN curl https://downloads.haskell.org/~ghcup/0.1.17.7/x86_64-linux-ghcup-0.1.17.7 -o $HOME/.local/bin/ghcup
RUN chmod a+x $HOME/.local/bin/ghcup

ENV PATH=$PATH:/root/.ghcup/bin/
RUN ghcup install ghc 9.10
RUN ghcup install cabal 3.12
RUN ghcup set ghc 9.10.1
RUN cabal update

ADD extra-vars /tmp/extra-vars

SHELL ["/bin/bash", "-c"]
CMD git clone $REPO \
    && cd $NAME \
    && git checkout $BASE_COMMIT \
    && cabal install ogma-cli:ogma \
    && ogma cfs --app-target-dir original --variable-file ogma-cli/examples/cfs-variables --variable-db ogma-cli/examples/cfs-variable-db --handlers-file ogma-cli/examples/cfs-handlers \
    && git checkout $COMMIT \
    && cabal install --overwrite-policy=always ogma-cli:ogma \
    && ogma cfs --app-target-dir updated --variable-file ogma-cli/examples/cfs-variables --variable-db ogma-cli/examples/cfs-variable-db --handlers-file ogma-cli/examples/cfs-handlers --template-vars /tmp/extra-vars \
    && sed -i -e 's/Make ICAROUS data/Make received data/g' original/fsw/src/copilot_cfs.c \
    && diff -rq original updated \
    && echo "Success"
```

Command (substitute variables based on new path after merge):
```sh
$ docker run -e "REPO=https://github.com/NASA/ogma" -e "NAME=ogma" -e "BASE_COMMIT=7f2525969ac5572e4486c697b60405d439ec3cb5" -e "COMMIT=<HASH>" -it ogma-verify-237
```

**Solution implemented**

Modify `copilot_cfs.h` header to make use of existing template variables.

Modify `copilot_cfs.c` to replace the specific includes with a new variable
that is expanded in place.

Modify `copilot_cfs.c` to remove ICAROUS from the comment documenting the
message handler function.

Modify `CMakeLists.txt` to replace the specific include of `Icarouslib` with an
additional variable that is expanded in place.

Document new variables in `ogma-cli`'s README.

**Further notes**

None.
  • Loading branch information
ivanperez-keera committed Feb 4, 2025
2 parents 7f25259 + 7080922 commit 846dcb0
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 13 deletions.
3 changes: 2 additions & 1 deletion ogma-cli/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
# Revision history for ogma-cli

## [1.X.Y] - 2025-02-03
## [1.X.Y] - 2025-02-04

* Add all auxiliary test files to distributable Cabal package (#216).
* Remove extraneous EOL character (#224).
* Update installation instructions to use cabal install (#149).
* Update README with new cFS template variables (#229).
* Expose handlers-file argument to cFS backend (#234).
* Expose template-vars argument to cFS backend (#106).
* Document new template variables in README (#237).

## [1.6.0] - 2025-01-21

Expand Down
13 changes: 13 additions & 0 deletions ogma-cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,19 @@ contains an object key `"APP_NAME"` with the value `"Monitor"`, a file or
directory in the template named `My_{{APP_NAME}}` will be expanded in the
destination directory as `My_Monitor`.
Using this flag, you can expand two more variables in the default template:
- `{{impl_extra_header}}`: contains a list of lines to add at the top of the
CFS application implementation, after the default includes. The value
assigned to this JSON key must be an array or list (each element will be
expanded in a separate line).
- `{{included_libraries}}`: contains a list of additional libraries that must
be included as dependencies in the `CMakeLists.txt` file for the moniroting
application in order to be able to compile it. The value assigned this JSON key
must be an array or list of dependencies, relative to the directory of the
monitoring app).
We understand that this level of customization may be insufficient for your
application. If that is the case, feel free to reach out to our team to discuss
how we could make the template expansion system more versatile.
Expand Down
3 changes: 2 additions & 1 deletion ogma-core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Revision history for ogma-core

## [1.X.Y] - 2025-02-03
## [1.X.Y] - 2025-02-04

* Import liftIO from Control.Monad.IO.Class (#215).
* Remove references to old design of Ogma from hlint files (#220).
Expand All @@ -11,6 +11,7 @@
* Simplify Copilot struct definitions by using generics (#199).
* Update cFS backend to process a handlers file (#234).
* Update cFS backend to process a template variables file (#106).
* Remove dependency on ICAROUS from generated cFS applications (#237).

## [1.6.0] - 2025-01-21

Expand Down
7 changes: 1 addition & 6 deletions ogma-core/src/Command/CFSApp.hs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ cFSApp targetDir mTemplateDir varNameFile varDBFile handlersFile
-- format).
varDBE <- E.try $
case varDBFile of
Nothing -> return knownVars
Nothing -> return []
Just fn -> fmap read <$> lines <$> readFile fn

handlersE <- parseOptionalRequirementsListFile handlersFile
Expand Down Expand Up @@ -144,11 +144,6 @@ cFSApp targetDir mTemplateDir varNameFile varDBFile handlersFile

return Success

-- | Predefined list of Icarous variables that are known to Ogma
knownVars :: [(String, String, String, String)]
knownVars =
[ ("position", "position_t", "ICAROUS_POSITION_MID", "IcarousPosition") ]

-- | Return the variable information needed to generate declarations
-- and subscriptions for a given variable name and variable database.
variableMap :: [(String, String, String, String)]
Expand Down
4 changes: 3 additions & 1 deletion ogma-core/templates/copilot-cfs/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ cmake_minimum_required(VERSION 2.6.4)
project(CFE_COPILOT_APP C)

include_directories(../../Modules/Core/Interfaces)
include_directories(../Icarouslib/fsw/platform_inc)
{{#included_libraries}}
include_directories({{{.}}})
{{/included_libraries}}
include_directories(../inc)
include_directories(fsw/mission_inc)
include_directories(fsw/platform_inc)
Expand Down
7 changes: 4 additions & 3 deletions ogma-core/templates/copilot-cfs/fsw/src/copilot_cfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@
#include "copilot_cfs_msg.h"
#include "copilot_cfs_events.h"
#include "copilot_cfs_version.h"
#include "Icarous_msgids.h"
#include "Icarous_msg.h"
{{#impl_extra_header}}
{{{.}}}
{{/impl_extra_header}}

{{#variables}}
{{varDeclType}} {{varDeclName}};
Expand Down Expand Up @@ -151,7 +152,7 @@ void COPILOT_ProcessCommandPacket(void)

{{#msgHandlers}}
/**
* Make ICAROUS data available to Copilot and run monitors.
* Make received data available to Copilot and run monitors.
*/
void COPILOT_Process{{msgDataDesc}}(void)
{
Expand Down
4 changes: 3 additions & 1 deletion ogma-core/templates/copilot-cfs/fsw/src/copilot_cfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@
void COPILOT_AppMain(void);
void COPILOT_AppInit(void);
void COPILOT_ProcessCommandPacket(void);
void COPILOT_ProcessIcarousPosition(void);
{{#msgCases}}
void COPILOT_Process{{msgInfoDesc}}(void);
{{/msgCases}}
void COPILOT_ResetCounters(void);

boolean COPILOT_VerifyCmdLength(CFE_SB_MsgPtr_t msg, uint16 ExpectedLength);
Expand Down

0 comments on commit 846dcb0

Please sign in to comment.