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

ogma-core: ROS backend incorrectly maps float, double type to std::float32 and std::float64 #138

Closed
ivanperez-keera opened this issue May 18, 2024 · 6 comments
Assignees
Labels
CR:Status:Closed Admin only: Change request that has been completed CR:Type:Bug Admin only: Change request pertaining to error detected
Milestone

Comments

@ivanperez-keera
Copy link
Member

ivanperez-keera commented May 18, 2024

Description*

The ROS backend is, by default, assuming that the data carried by std_msgs::msg::Float32 is of type float32, when it appears to be just a float. The same is happening for Float64, float64 and double. The code currently produced for those cases does not compile.

The type mappings are described in http://docs.ros.org/en/humble/Concepts/Basic/About-Interfaces.html, which states that, in C++, float and double should be used.

Type

  • Bug: backend generates incorrect code.

Additional context

None.

Requester

  • Alexander Will and Robert Grizzard (VCU)

Method to check presence of bug

Compiling a specification with an input of type Float or Double results in code that will not compile:

--- ROS.hs
import Copilot.Compile.C99
import Copilot.Language
import Language.Copilot    (reify)
import Prelude             hiding (not, (>=))

inputSignal :: Stream Float
inputSignal = extern "input_signal" Nothing

propTestCopilot :: Stream Bool
propTestCopilot = inputSignal >= 5

spec :: Spec
spec = do
  trigger "handlerTestCopilot" (not propTestCopilot) []

main :: IO ()
main = reify spec >>= compileWith settings "monitor"
  where
    settings = mkDefaultCSettings
                 { cSettingsOutputDirectory = "demo/copilot/src/" }
--- vars-db
("input_signal","float","/demo/topic","float")
$ ogma ros --app-target-dir demo --variable-db vars-db --variable-file ogma-cli/examples/ros-copilot/variables --handlers-file ogma-cli/examples/ros-copilot/handlers
$ cabal v1-exec -- runhaskell ROS.hs
$ cd demo/
$ docker build  .
Sending build context to Docker daemon  13.31kB
Step 1/12 : FROM osrf/space-ros:latest
 ---> 3737e04f8b7c
Step 2/12 : ARG USER=spaceros-user
 ---> Running in 000def0bb6b9
Removing intermediate container 000def0bb6b9
 ---> 501fe9476618
Step 3/12 : ARG PACKAGE_PATH=/home/${USER}/monitors
 ---> Running in 6a1ea90b9f15
Removing intermediate container 6a1ea90b9f15
 ---> 9a76021069e9
Step 4/12 : ARG ROS_PATH=/home/${USER}/spaceros/
 ---> Running in a2475046971b
Removing intermediate container a2475046971b
 ---> 9b55d23d5e4c
Step 5/12 : RUN mkdir -p ${PACKAGE_PATH}/src/
 ---> Running in 8450449dcbc9
Removing intermediate container 8450449dcbc9
 ---> ac553c5a0497
Step 6/12 : ADD copilot ${PACKAGE_PATH}/src/copilot
 ---> 79264c5d486b
Step 7/12 : USER root
 ---> Running in fb3120dfb81c
Removing intermediate container fb3120dfb81c
 ---> fc106f45d0aa
Step 8/12 : RUN chown -R ${USER} ${PACKAGE_PATH}
 ---> Running in e8d1c031d73a
Removing intermediate container e8d1c031d73a
 ---> ae8e465b31d5
Step 9/12 : USER ${USER}
 ---> Running in e295eb65f452
Removing intermediate container e295eb65f452
 ---> c1b9ff3b0ca5
Step 10/12 : SHELL ["/bin/bash", "-c"]
 ---> Running in 0d9932c678a2
Removing intermediate container 0d9932c678a2
 ---> 563d710e981f
Step 11/12 : WORKDIR ${PACKAGE_PATH}
 ---> Running in a29cd3f6a93b
Removing intermediate container a29cd3f6a93b
 ---> f15020c12cd9
Step 12/12 : RUN source ${ROS_PATH}/install/setup.bash &&     colcon build
 ---> Running in 1a9a8a98a8a9
Starting >>> copilot
--- stderr: copilot
/home/spaceros-user/monitors/src/copilot/src/copilot_monitor.cpp:25:6: error: ‘float32’ in namespace ‘std’ does not name a type; did you mean ‘float_t’?
   25 | std::float32 input_signal;
      |      ^~~~~~~
      |      float_t
gmake[2]: *** [CMakeFiles/copilot.dir/build.make:76: CMakeFiles/copilot.dir/src/copilot_monitor.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:137: CMakeFiles/copilot.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
---
Failed   <<< copilot [16.4s, exited with code 2]

Summary: 0 packages finished [18.4s]
  1 package failed: copilot
  1 package had stderr output: copilot
The command '/bin/bash -c source ${ROS_PATH}/install/setup.bash &&     colcon build' returned a non-zero code: 2

Expected result

The execution above should produce a Copilot specification that can be compiled using docker.

Desired result

The execution above should produce a Copilot specification that can be compiled using docker.

Proposed solution

Modify the ROS backend to map the contents of Float32 messages to float, and variables of type float (in the vars DB) to just float in C++.

Further notes

None.

@ivanperez-keera ivanperez-keera added CR:Status:Initiated Admin only: Change request that has been initiated CR:Type:Bug Admin only: Change request pertaining to error detected labels May 21, 2024
@ivanperez-keera
Copy link
Member Author

Change Manager: Confirmed that the issue exists.

@ivanperez-keera ivanperez-keera added CR:Status:Confirmed Admin only: Change request that has been acknowledged by the change manager and removed CR:Status:Initiated Admin only: Change request that has been initiated labels May 21, 2024
@ivanperez-keera
Copy link
Member Author

Technical Lead: Confirmed that the issue should be addressed.

@ivanperez-keera ivanperez-keera added CR:Status:Accepted Admin only: Change request accepted by technical lead and removed CR:Status:Confirmed Admin only: Change request that has been acknowledged by the change manager labels May 21, 2024
@ivanperez-keera
Copy link
Member Author

Technical Lead: Issue scheduled for fixing in Ogma 1.4.

Fix assigned to: @ivanperez-keera.

@ivanperez-keera ivanperez-keera added CR:Status:Scheduled Admin only: Change requested scheduled and removed CR:Status:Accepted Admin only: Change request accepted by technical lead labels May 21, 2024
@ivanperez-keera ivanperez-keera added this to the 1.4.0 milestone May 21, 2024
@ivanperez-keera ivanperez-keera self-assigned this May 21, 2024
@ivanperez-keera ivanperez-keera added CR:Status:Implementation Admin only: Change request that is currently being implemented and removed CR:Status:Scheduled Admin only: Change requested scheduled labels May 21, 2024
ivanperez-keera added a commit that referenced this issue May 21, 2024
The ROS backend is, by default, assuming that the data carried by
std_msgs::msg::Float32 is of type float32, when it appears to be just a
float. The same is happening for Float64, float64 and double. The code
currently produced for those cases does not compile.

The type mappings are described in
http://docs.ros.org/en/humble/Concepts/Basic/About-Interfaces.html,
which states that, in C++, float and double should be used.

This commit modifies the ROS backend so that it expects global variables
of input types `float` and `double` in the variable DB to have the types
`float` and `double`, respectively, in the generated C++ code.
ivanperez-keera added a commit that referenced this issue May 22, 2024
…#138.

This commit modifies an example to use also an input variable of type
float and one of type double, both of which were failing in the ROS
backend.

The example in question is currently being used by a CI script to test
the ROS backend, making the new example serve as a regression test.
@ivanperez-keera
Copy link
Member Author

Implementor: Solution implemented, review requested.

@ivanperez-keera ivanperez-keera added CR:Status:Verification Admin only: Change request that is currently being verified and removed CR:Status:Implementation Admin only: Change request that is currently being implemented labels May 22, 2024
@ivanperez-keera
Copy link
Member Author

Change Manager: Verified that:

@ivanperez-keera
Copy link
Member Author

Change Manager: Implementation ready to be merged.

@ivanperez-keera ivanperez-keera added CR:Status:Closed Admin only: Change request that has been completed and removed CR:Status:Verification Admin only: Change request that is currently being verified labels May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR:Status:Closed Admin only: Change request that has been completed CR:Type:Bug Admin only: Change request pertaining to error detected
Projects
None yet
Development

No branches or pull requests

1 participant