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.

Sign up for GitHub

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

Jump to bottom

Add abstraction layer for external function of Streams.error #4496

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add abstraction layer for external function of Streams.error #4496

wants to merge 1 commit into from

Conversation

beutlich
Copy link
Member

This avoids that Modelica utility function ModelicaError has to be provided by library ModelicaExternalC.

@beutlich beutlich added L: C-Sources Issue addresses Modelica/Resources/C-Sources L: Utilities Issue addresses Modelica.Utilities labels Oct 27, 2024
@casella
Copy link
Contributor

casella commented Nov 11, 2024

@beutlich can you quickly describe what is the advantage of this solution? Thanks!

@beutlich
Copy link
Member Author

@beutlich can you quickly describe what is the advantage of this solution? Thanks!

Currently MSL requires that tools provide a library (static or dynamic) named "ModelicaExternalC.*" where symbol "ModelicaError" gets resolved. However, this is not required from MLS perspective. This PR fixes this superfluous requirement.

@beutlich beutlich force-pushed the update-streams-error branch from bdf4e0b to fd3966e Compare November 11, 2024 19:58
@casella
Copy link
Contributor

casella commented Nov 11, 2024

Is this related to #4476? If so, we should probably try to get a harmonized solution. Or is this an orthogonal issue?

HansOlsson
HansOlsson approved these changes Nov 14, 2024
View reviewed changes
Copy link
Contributor

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

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

Ok, makes sense.

@beutlich
Copy link
Member Author

Is this related to #4476? If so, we should probably try to get a harmonized solution. Or is this an orthogonal issue?

I'd rather consider this an orthogonal issue. The current external function of Stream.error uses some kind of short-hand of ModelicaError being exposed of library ModelicaExternalC. However ModelicaExternalC is provided by MSL (and not specified in MLS) whereas ModelicaError is provided by vendor-specific ModelicaUtilities.

@beutlich beutlich force-pushed the update-streams-error branch from fd3966e to 0812a07 Compare November 16, 2024 09:39
@beutlich
Add abstraction layer for external function of Streams.error
64dc67b 
This avoids that Modelica utility function ModelicaError has to be provided by library ModelicaExternalC.
@beutlich beutlich force-pushed the update-streams-error branch from 0812a07 to 64dc67b Compare December 28, 2024 10:57
@beutlich beutlich mentioned this pull request Dec 28, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HansOlsson HansOlsson HansOlsson approved these changes

@casella casella Awaiting requested review from casella

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
L: C-Sources Issue addresses Modelica/Resources/C-Sources L: Utilities Issue addresses Modelica.Utilities
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@beutlich @casella @HansOlsson