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

Simulink integration #445

Merged
merged 83 commits into from
Jul 17, 2021
Merged

Simulink integration #445

merged 83 commits into from
Jul 17, 2021

Conversation

sthelia
Copy link
Contributor

@sthelia sthelia commented Jun 16, 2021

I've been working on improving an S-function to be able to integrate JSBSim with Simulink.

I included an example Simulink file of running one of the 737 Scripts. It should work for both Windows and Linux users.

sthelia and others added 3 commits June 16, 2021 15:16
Including interface files and S-function files to be able to run JSBSim from Simulink.
The setup files JSBSimSImulinkCompile and clearSF are also included.
A short readme files includes instructions on how to run it in Simulink.
@bcoconni
Copy link
Member

Thanks for the PR @sthelia 👍

As per our discussion I have just pushed a commit (sthelia/jsbsim@d0cf1e2) to your PR to trigger the build of the S-Function by our CI/CD workflow (GitHub Actions).

This commit also allows the S-Function to be built along with the JSBSim C++ library: for that you need to pass -DBUILD_MATLAB_SFUNCTION=ON to CMake.

Copy link
Member

@bcoconni bcoconni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution.

There are a number of minor modifications which need to be addressed before merging this PR. Most of them are related to the fact that C++ uses true and false for booleans (not 0 and 1).

If you agree with a suggestion, just press the button Commit suggestion and GitHub will take care of updating your code on your branch.

matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
sthelia and others added 25 commits June 21, 2021 10:47
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
Copy link
Member

@bcoconni bcoconni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sthelia I have initiated a new review regarding the problem you have reported about the S-Function not cleaning up after itself.

If you feel more comfortable with a commit review, I have compiled all the changes I am suggesting in bcoconni/jsbsim@0d1410b. Unfortunately, I don't have access to Matlab so you'll have to check yourself whether or not the changes I have suggested allow avoiding calling clearSF.m after each Simulink run. All I could check with gitHub Actions was that the code compiles successfully and that the example ex737cruise.slx runs successfully as well.

I might also push the changes directly to your PR: if you prefer so, just let me know.

matlab/JSBSim_SFunction.cpp Outdated Show resolved Hide resolved
matlab/JSBSim_SFunction.cpp Outdated Show resolved Hide resolved
matlab/JSBSim_SFunction.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.cpp Outdated Show resolved Hide resolved
matlab/JSBSimInterface.h Outdated Show resolved Hide resolved
//fdmExec = 0L;
//JSBSim::~FGFDMExec();
//delete ic;
delete fdmExec;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you have made FGFDMExec a member of JSBSimInterface, you can call delete on fdmExec

@bcoconni
Copy link
Member

@sthelia Also please include an open source license to all files in this PR otherwise we won't be able to pull them to JSBSim.

@agodemar
Copy link
Contributor

agodemar commented Jul 3, 2021

@bcoconni Do someone of us admins have to "Squash and merge" this PR?

@seanmcleod
Copy link
Member

@agodemar it looks like @bcoconni is waiting for feedback on his suggested changes for the S-function cleanup issue and an open source license for the code.

@sthelia
Copy link
Contributor Author

sthelia commented Jul 3, 2021

I am currently on vacation and will not have access to my computer until next week - hope to test everything you have suggested then!

@bcoconni
Copy link
Member

bcoconni commented Jul 3, 2021

I am currently on vacation and will not have access to my computer until next week - hope to test everything you have suggested then!

@sthelia No rush 😄 JSBSim has been lacking an S-Function for years, it can still wait for a few more days/weeks !
Enjoy your vacation !

@bcoconni
Copy link
Member

bcoconni commented Jul 3, 2021

@agodemar If you are eager to test the S-Function code, you can pull the complete code from my fork: bcoconni/jsbsim@0d1410b (branch pr/sthelia/445). GitHub Actions report that it compiles and executes the example ex737cruise.slx successfully.

Co-authored-by: Bertrand Coconnier <bcoconni@users.noreply.github.com>
@sthelia
Copy link
Contributor Author

sthelia commented Jul 8, 2021

@sthelia Also please include an open source license to all files in this PR otherwise we won't be able to pull them to JSBSim.

What should be included? Is it the copyright text mentioning GNU that is added on top on the other source files?

Cleaning up some unused functions and implementing suggested changes.
@sthelia
Copy link
Contributor Author

sthelia commented Jul 8, 2021

Thank you @bcoconni for the help with the C++ sillyness fixes, I am more than happy to not have to understand why they happen :D I also appreciate the comments to improve and cleanup the code, I am definitely learning a lot from this.

@bcoconni
Copy link
Member

What should be included? Is it the copyright text mentioning GNU that is added on top on the other source files?

Well, you've entered the realm of licenses which is always a joy when dealing with a mixture of open source (JSBSim and your code) and proprietary (Matlab) licenses. From what I could gather, you stood on the shoulder of giants 😉 to write this code so we need to give them proper credits:

Copyright (c) 2009, Brian Mills
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are met:

* Redistributions of source code must retain the above copyright notice, this
  list of conditions and the following disclaimer.

* Redistributions in binary form must reproduce the above copyright notice,
  this list of conditions and the following disclaimer in the documentation
  and/or other materials provided with the distribution
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE
FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

We also need to ask @podhrmic (Michal Podhradsky) for permission to use his code under the license above since you explicitly mentioned Michal's code as a reference.

Finally, Brian Mills also mentioned @agodemar's work (Agostino De Marco) as the source for his original code but I guess this permission should be quite easy to get 😉

@podhrmic and @agodemar, just drop a message below in this PR and I guess this would do as an evidence.

Once we've got due permission, you can copy the license term at the top of each file in this PR:

/*
Copyright (c) 2009, Brian Mills
All rights reserved.

Copyright (c) 2021, Agostino De Marco, Michal Podhradsky, Tilda Sikström

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are met:

* Redistributions of source code must retain the above copyright notice, this
  list of conditions and the following disclaimer.

* Redistributions in binary form must reproduce the above copyright notice,
  this list of conditions and the following disclaimer in the documentation
  and/or other materials provided with the distribution
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE
FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

@bcoconni
Copy link
Member

Regarding the files that need a license term, I'd say that you can limit yourself to the following non trivial files:

  • matlab/JSBSimInterface.cpp
  • matlab/JSBSimInterface.h
  • matlab/JSBSim_SFunction.cpp
  • matlab/readme.txt

The other files are either build files or trivial files so you can save yourself the trouble of putting a license in them.

@bcoconni
Copy link
Member

Oh I just saw that @podhrmic gave credit to @EliaTarasov in his repo then we also need Elia's permission.

@bcoconni bcoconni mentioned this pull request Jul 10, 2021
@agodemar
Copy link
Contributor

Finally, Brian Mills also mentioned @agodemar's work (Agostino De Marco) as the source for his original code but I guess this permission should be quite easy to get 😉

@podhrmic and @agodemar, just drop a message below in this PR and I guess this would do as an evidence.

You have my permission. All this effort, up to this PR by @sthelia began from my original code back in 2009.

@EliaTarasov
Copy link
Contributor

Feel free to use it, @bcoconni :)

@podhrmic
Copy link
Contributor

Hi @bcoconni - you have my permission to use my code:-)

@sthelia
Copy link
Contributor Author

sthelia commented Jul 16, 2021

Alright, the license term is added.

Thanks again everyone who developed the code I based this on, I would not have been able to present this pull request without it!

@bcoconni bcoconni merged commit d5a7a71 into JSBSim-Team:master Jul 17, 2021
@bcoconni
Copy link
Member

The PR is merged ! JSBSim now has its official interface with Matlab.
Thanks @sthelia for your commitment and bringing this contribution to its positive conclusion.
Thanks also to @agodemar, @EliaTarasov, @podhrmic for their work without which this PR would not exist today.

@bcoconni bcoconni mentioned this pull request Jul 17, 2021
4 tasks
sthagen added a commit to sthagen/JSBSim-Team-jsbsim that referenced this pull request Jul 17, 2021
@agodemar
Copy link
Contributor

The PR is merged ! JSBSim now has its official interface with Matlab.
Thanks @sthelia for your commitment and bringing this contribution to its positive conclusion.
Thanks also to @agodemar, @EliaTarasov, @podhrmic for their work without which this PR would not exist today.

This is great news. Thank you @sthelia for your effort!

bcoconni pushed a commit to bcoconni/jsbsim that referenced this pull request Jul 18, 2021
Including interface files and S-function files to be able to run JSBSim from Simulink.
The setup files JSBSimSImulinkCompile and clearSF are also included.
A short readme files includes instructions on how to run it in Simulink.


Co-authored-by: Agostino De Marco <agostino.demarco@unina.it>
Co-authored-by: Elia Tarasov <elias.tarasov@gmail.com>
Co-authored-by: Michal Podhradsky <mpodhradsky@galois.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants