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

Implement simple battery (see issue #2957) #2959

Merged
merged 3 commits into from
Jun 22, 2019
Merged

Implement simple battery (see issue #2957) #2959

merged 3 commits into from
Jun 22, 2019

Conversation

AHaumer
Copy link
Contributor

@AHaumer AHaumer commented Jun 6, 2019 •
edited by beutlich

Fix #2957.

@AHaumer AHaumer added enhancement New feature or enhancement L: Electrical.Analog Issue addresses Modelica.Electrical.Analog P: low Low priority issue labels Jun 6, 2019
@AHaumer AHaumer added this to the MSL4.0.0 milestone Jun 6, 2019
@AHaumer AHaumer self-assigned this Jun 6, 2019
@AHaumer AHaumer mentioned this pull request Jun 6, 2019
Closed
@AHaumer
addressing concerns of @christiankral regarding OCV linearly dependen…
6cf8c29 
…t on SOC with Boolean parameter useLinearSOCDependency=true
@beutlich beutlich changed the title implement simple battery (see issue #2957) Implement simple battery (see issue #2957) Jun 13, 2019
christophclauss
christophclauss approved these changes Jun 16, 2019
View reviewed changes
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.

This enhancement is fine to me. It is out of doubt that battery models are needed, especially models that non-experts can understand.
The place in the MSL is exactly that place I'd also had chosen. I like a flat hierarchy. Too deeply sorted models do not give a convenient overview.
As far as I understand this matter the models are correct. Later on I will do further tests, but this should not prevent from approving this PR.
Many thanks for developing these models.

christiankral
christiankral reviewed Jun 21, 2019
View reviewed changes
Copy link
Contributor

@christiankral christiankral left a comment •
edited

Choose a reason for hiding this comment

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

Please re-request my review after reviewing AHaumer#1

@christiankral @AHaumer
Change default and example values of OCVmin (#1)
b44f377 
* Change hard to soft default value

OCVmin = 0 is not a reasonable value in a real applications so it's better to use a start value or to omit the default value at all

* Set simulation value  of OCVmin to 10V 

OCVmin = 0 is not realistic in an application
@christiankral christiankral self-requested a review June 22, 2019 09:01
@beutlich beutlich merged commit e57c5a6 into modelica:master Jun 22, 2019
@beutlich beutlich assigned beutlich and unassigned AHaumer Jun 22, 2019
@beutlich beutlich mentioned this pull request Jun 22, 2019
Merged
@AHaumer AHaumer deleted the 2957 branch July 8, 2019 07:20
@beutlich beutlich added L: Electrical.Batteries Issue addresses Modelica.Electrical.Batteries and removed L: Electrical.Analog Issue addresses Modelica.Electrical.Analog labels Jul 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christiankral christiankral christiankral approved these changes

@christophclauss christophclauss christophclauss approved these changes

Assignees

@beutlich beutlich

Labels
enhancement New feature or enhancement L: Electrical.Batteries Issue addresses Modelica.Electrical.Batteries P: low Low priority issue
Projects
None yet
Milestone
MSL4.0.0
Development

Successfully merging this pull request may close these issues.

Implement idealized battery
4 participants
@AHaumer @christiankral @christophclauss @beutlich