Skip to content

Conversation

@henrikt-ma
Copy link
Contributor

No description provided.

@henrikt-ma henrikt-ma added the L: Electrical.Batteries Issue addresses Modelica.Electrical.Batteries label Apr 6, 2023
@beutlich beutlich added the example Issue only addresses example(s) label Apr 17, 2023
Copy link
Contributor

@AHaumer AHaumer left a comment

Choose a reason for hiding this comment

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

I'd suggest the following change:
parameter Modelica.Units.SI.Current Isc = 1200 "Short-circuit current of cell at OCVmax";
Why don't make this parameter public?

henrikt-ma and others added 3 commits May 25, 2023 12:20
Co-authored-by: Hans Olsson <[email protected]>
Co-authored-by: Hans Olsson <[email protected]>
@henrikt-ma henrikt-ma requested a review from AHaumer May 25, 2023 10:22
@HansOlsson
Copy link
Contributor

Coordinate as described in #4101

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.

@henrikt-ma I suggest to remove public, see suggestion.

Other than that, I am in favor of approving this proposal.

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.

OCVmax and Isc are now considered in each battery example

Co-authored-by: Hans Olsson <[email protected]>
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.

Fine by me - I think it is better to wait for actual expert approval; and I haven't figured out how to remove myself as reviewer.

@hubertus65 hubertus65 removed the request for review from AHaumer November 14, 2023 14:26
Copy link
Member

@hubertus65 hubertus65 left a comment

Choose a reason for hiding this comment

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

Looks good, and approved in MAP-LIB meeting

@hubertus65 hubertus65 dismissed AHaumer’s stale review November 14, 2023 14:31

This is outdated and fixed as suggested.

@hubertus65 hubertus65 merged commit 402662a into modelica:master Nov 14, 2023
@beutlich beutlich added this to the MSL4.1.0 milestone Nov 14, 2023
@henrikt-ma henrikt-ma deleted the cccvcharging-unit-error branch December 12, 2023 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

example Issue only addresses example(s) L: Electrical.Batteries Issue addresses Modelica.Electrical.Batteries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants