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

Fix for #3971 #4074

Merged
merged 2 commits into from
Mar 7, 2023
Merged

Conversation

christoff-buerger
Copy link
Member

Fix for #3971: Moved the parameters into the interface. I double checked that all interface implementations (subclasses) in fact introduce both parameters; their defaults have been the same.

Co-authored-by: Thomas Beutlich <modelica@tbeu.de>
@HansOlsson
Copy link
Contributor

Hm... I did suggest that solution, and it makes sense - but it is a bit inconsistent since for non-clocked signals Modelica.Blocks.Interfaces.PartialNoise is inherited by Modelica.Blocks.Noise.UniformNoise which is unlimited.

If we think that is a problem we could have PartialRangeLimitedNoise as a separate base-class containing these parameters in both cases.

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.

I think this makes sense, and have suggested an alternative if we want to have more consistent interfaces.

@christoff-buerger
Copy link
Member Author

Hm... I did suggest that solution, and it makes sense - but it is a bit inconsistent since for non-clocked signals Modelica.Blocks.Interfaces.PartialNoise is inherited by Modelica.Blocks.Noise.UniformNoise which is unlimited.

If we think that is a problem we could have PartialRangeLimitedNoise as a separate base-class containing these parameters in both cases.

Yes, we can do that; I have been aware of this alternative when pushing the current proposal. My thinking was along the lines, that such an additional base-class for even more variety can be done if there is any need at hand (hence, users request); I don't want to model too much that likely is never used as far as we can see.

@christoff-buerger christoff-buerger merged commit ef08d88 into modelica:master Mar 7, 2023
@beutlich beutlich added this to the MSL4.1.0 milestone Mar 7, 2023
@beutlich beutlich changed the title Fix for #3971. Fix for #3971 Mar 11, 2023
@beutlich beutlich added the L: Clocked Issue addresses Modelica.Clocked label Mar 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: Clocked Issue addresses Modelica.Clocked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants