search

open-simulation-platform / libcosim

OSP C++ co-simulation library
https://open-simulation-platform.github.io/libcosim
Mozilla Public License 2.0
55 stars 10 forks source link

Non URI file scheme equal type name support #688

Closed msteinsto closed 2 years ago

msteinsto commented 2 years ago

Closes #685 by allowing the OspModelDescription file to be located in a different directory than the OspSystemStructure file togheter with the model fmu. A new function for parsing a query file parameter is added for non file URI schemes with a fallback of the existing function. Both relative and absolute path is supported. This has previously only been possible for local simulations.

kyllingstad commented 2 years ago

Looks good to me too. The only thing I'm wondering is whether file_query_uri_to_path() belongs in uri.hpp. The rest of that header AFAICT is all generic, standard URI-related functionality. The new function seems to be more of an OSP-specific thing which is only used in the OSP config parser. Perhaps it should be moved to osp_config_parser.cpp as an internal implementation detail?

msteinsto commented 2 years ago

Looks good to me too. The only thing I'm wondering is whether file_query_uri_to_path() belongs in uri.hpp. The rest of that header AFAICT is all generic, standard URI-related functionality. The new function seems to be more of an OSP-specific thing which is only used in the OSP config parser. Perhaps it should be moved to osp_config_parser.cpp as an internal implementation detail?

Depends if the function is expected to be reused later outside osp_config_parser.cpp. I can currently find one other potential use case in a refactored proxy_uri_sub_resolver::lookup_model. As of now I agree it is more a OSP-specific thing and belongs in osp_config_parser.cpp. Inputs?

kyllingstad commented 2 years ago

I'd look at it the other way around. If we later find that the function is useful elsewhere, it can easily be moved to the public API then. But once a function is in the public API, it's difficult to change or get rid of, so we shouldn't add it prematurely.