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

Rename base classes with support flange #3362

Open
tobolar opened this issue Jan 23, 2020 · 8 comments
Open

Rename base classes with support flange #3362

tobolar opened this issue Jan 23, 2020 · 8 comments
Labels
discussion Discussion issue that it not necessarily related to a concrete bug or feature L: Mechanics.Rotational Issue addresses Modelica.Mechanics.Rotational L: Mechanics.Translational Issue addresses Modelica.Mechanics.Translational

Comments

@tobolar
Copy link
Contributor

tobolar commented Jan 23, 2020

Due to similar model background, the following clases in Modelica.Mechanics.Rotational.Interfaces should be renamed:

  • PartialElementaryOneFlangeAndSupport2 to PartialOneFlangeAndSupportElementary (i.e. a counterpart of PartialOneFlangeAndSupport)
  • PartialElementaryTwoFlangesAndSupport2 to PartialTwoFlangesAndSupportElementary (i.e. a counterpart of PartialTwoFlangesAndSupport)

May be there is a better option for the supplement instead of "Elementary".

This concerns Modelica.Mechanics.Translational.Interfaces accordingly.

@tobolar tobolar added L: Mechanics.Rotational Issue addresses Modelica.Mechanics.Rotational L: Mechanics.Translational Issue addresses Modelica.Mechanics.Translational discussion Discussion issue that it not necessarily related to a concrete bug or feature labels Jan 23, 2020
@tobolar tobolar added this to the MSL4.0.0 milestone Jan 23, 2020
@beutlich
Copy link
Member

We could rename it to PartialElementaryOneFlangeAndSupport/PartialElementaryTwoFlangesAndSupport (by simply removing the trailing "2"), but then there is some risk that someone using PartialElementaryOneFlangeAndSupport/PartialElementaryTwoFlangesAndSupport from MSL v3.2.3, who uses MSL v4.0.0 without conversion suddenly uses the wrong interface model.

@beutlich
Copy link
Member

Here's the commit were the "2" models have been introduced: 68c3c51

@dietmarw
Copy link
Member

Why not move non-2 ones to ObsoleteModelica4 and rename the 2-models? That way it should not break the interfaces.

@beutlich
Copy link
Member

That's what already happened by 5b13d04. If we now introduce new models with that same name we might risk some name conflicts if conversion is either missed or run twice.

@dietmarw
Copy link
Member

If we assume a conversion does not take correctly place then we have a whole lot of other problems. It would be strange to workaround that in this case. I'm sure if you look at this like that then we have a number of other cases that also need to be considered. I think it is safe to say that if conversion fails or is not done when using MSL 4.0.0 then user models will break.

@tobolar
Copy link
Contributor Author

tobolar commented Jan 24, 2020

My intension is to give similar names to similar classes. Thus, the package content would be:

...
PartialOneFlangeAndSupport
PartialOneFlangeAndSupportElementary <-- PartialElementaryOneFlangeAndSupport2
PartialTwoFlangesAndSupport
PartialTwoFlangesAndSupportElementary <-- PartialElementaryTwoFlangesAndSupport2
...

I'm aware of the risk by leaving "2" only. This is not what I'm addressing here.

@beutlich
Copy link
Member

beutlich commented Jan 24, 2020

But there still is PartialElementaryRotationalToTranslational, making it inconsistent.

@tobolar
Copy link
Contributor Author

tobolar commented Jan 24, 2020

Hmm. This is true.
Actually, this is even more confusing as PartialElementaryRotationalToTranslational has the same structure like PartialOneFlangeAndSupport or PartialTwoFlangesAndSupport. But it is documented to be intended for "equations in the text layer" and so is really used e.g. in IdealGearR2T.
This additonally means:

  1. PartialElementaryRotationalToTranslational shall be renamed to PartialRotationalToTranslational because this class uses internalSupport(s) and shall thus indeed be used for graphically given components. Even if such class would not be used anywhere in MSL now, it is necessary for proper conversion.
  2. There should be defined a new class PartialRotationalToTranslationalElementary used for IdealGearR2T.

This is a lot more work but is consistent.

@beutlich beutlich removed this from the MSL4.0.0 milestone Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion issue that it not necessarily related to a concrete bug or feature L: Mechanics.Rotational Issue addresses Modelica.Mechanics.Rotational L: Mechanics.Translational Issue addresses Modelica.Mechanics.Translational
Projects
None yet
Development

No branches or pull requests

3 participants