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

Add support for running scenarios with locally-defined services #367

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AshFungor
Copy link
Contributor

Hey, @riebl!

I've realized that problem with libraries & ned folders originally was caused by the placement of script's generator function. If placed after processing scenarios, all dependencies are detected and formatted just fine. Although, I wonder if it is a good idea to put all services into one variable globally.

Special file with parameters for each scenario is a great alternative, but it seems like a significant overhead and might confuse people too much so they stop using this script completely.

What do you think?

@AshFungor
Copy link
Contributor Author

On second thought - we might just add current working directory of a running scipt (parent directory of omnetpp.ini) to ned folders. Libraries will be added globally (you cannot define targets with same names in CMake, anyway).

This should also work.

Copy link
Owner

@riebl riebl left a comment

Choose a reason for hiding this comment

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

I like this draft and it avoids my too complicated over-engineering :-)

@AshFungor
Copy link
Contributor Author

Hey, @riebl!

Apart from your suggestions I also added .flake8 formatting settings (considering we now have extensive amount of python code, some formatting seems reasonable).

My only problem with this PR are install targets. Not all libraries & ned folders are getting installed, and with multi-config generators I cannot think of a way to generate options in uniform way. I would like to know your thoughts on this :)

I removed installation of runner script and launch configs temporary.

@AshFungor AshFungor requested a review from riebl February 21, 2025 18:45
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.

2 participants