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

resolve #2116 Sources with variable frequency #3049

Merged
merged 8 commits into from
Jul 17, 2019

Conversation

AHaumer
Copy link
Contributor

@AHaumer AHaumer commented Jul 15, 2019

This is a resolution to fix #2116
Before doing that, I removed the "encapsulated" from 3 examples (don't know why that was used).
To implement the voltage and current source with the same icon, I moved the icons form the partial models to a new Icon package.
Implementation is robust. Parameter startTime omitted, since this affects the initial phase shift (which can be initialized anyway) and if the sine should start later, you could keep the amlitude to zero until the sine should start.
To test the sources, there are 2 new examples.

@AHaumer AHaumer added enhancement New feature or enhancement L: Electrical.Analog Issue addresses Modelica.Electrical.Analog labels Jul 15, 2019
@AHaumer AHaumer added this to the MSL4.0.0 milestone Jul 15, 2019
@beutlich beutlich removed the request for review from kristinmajetta July 15, 2019 19:18
Copy link
Member

@beutlich beutlich left a comment

Choose a reason for hiding this comment

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

Should the sources from Modelica.Electrical.Analog.Sources (for example SignalVoltage) also inherit from the new source icons?

@beutlich beutlich self-assigned this Jul 15, 2019
@AHaumer
Copy link
Contributor Author

AHaumer commented Jul 15, 2019

@beutlich As far as I see, all sources inherit from partial VoltageSource or CurrentSource, and these inherit from the same icons. So the icons are defined only once.
The partial VoltageSource or CurrentSource cannot bs used for the new variable frequency sources, since the replaceable signalSource is of no use for variable frequency.

Copy link

@christophclauss christophclauss left a comment

Choose a reason for hiding this comment

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

It is o.k. to me.

@AHaumer
Copy link
Contributor Author

AHaumer commented Jul 16, 2019

Just wait a little bit with merging,
I'll provide the signal source block, too.

…amplitude

implemented sine signal source with variable frequency and amplitude
@AHaumer
Copy link
Contributor Author

AHaumer commented Jul 16, 2019

Just wait an hour or two, I'll provide two nice examples.

Copy link
Contributor

@christiankral christiankral left a comment

Choose a reason for hiding this comment

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

OK for me

Copy link
Member

@beutlich beutlich left a comment

Choose a reason for hiding this comment

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

Two questions:

  1. Why is only a new sine block added, but no cosine block?
  2. Should SineVariableFrequencyAndAmplitude really go to Blocks.Sources, since it is the only block having inputs? Could also go to Blocks.Math.

@AHaumer
Copy link
Contributor Author

AHaumer commented Jul 16, 2019

@beutlich It's more a source. You may define frequency and amplitude constant, or prescribe them via inputs.

@AHaumer
Copy link
Contributor Author

AHaumer commented Jul 16, 2019

@beutlich There's also only an ExpSine, not ExpCosine.
With ExpSine you achie an ExpCosine by phase = pi/2.
In SineVariableFrequencyAndAmplitude you achieve a cosine by choosing the initial value of the angle (state).
BTW, may I propose a sinc-source (sin(x))/x ? It's important when dealing with signals ...

@AHaumer
Copy link
Contributor Author

AHaumer commented Jul 16, 2019

I've also improved the icon.

@AHaumer
Copy link
Contributor Author

AHaumer commented Jul 17, 2019

I've added cosine with variable frequency and variable amplitude

@AHaumer AHaumer merged commit a31662e into modelica:master Jul 17, 2019
@beutlich beutlich removed their assignment Jul 17, 2019
@AHaumer AHaumer deleted the 2116VarFreSource branch July 18, 2019 13:41
beutlich pushed a commit that referenced this pull request Aug 23, 2019
beutlich pushed a commit that referenced this pull request Aug 23, 2019
beutlich added a commit that referenced this pull request Aug 23, 2019
beutlich added a commit that referenced this pull request Aug 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement L: Electrical.Analog Issue addresses Modelica.Electrical.Analog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sine sources with varying frequency
4 participants