Tracking S231P public review comments

anandkp92 commented 6 months ago

[ ] #147
[ ] How to distinguish between CDL blocks and Modelica models? (add this to the specification)
[ ] Sequence names include package names? e.g: a S231P:ElementaryBlock . (add this to specification)
[ ] How to add a label/source of version and tool that generated CXF
[ ] Add support for replaceable in CDL: https://github.com/lbl-srg/obc/pull/146
[ ] The enumeration must be defined, such as for ZeroTime and for the PID controller.

anandkp92 commented 5 months ago

[ ] how to handle final class_modification (eg: in this example, how do we handle gain(final k = k)?
[ ] how to handle class modifications that are not part of the standard? (eg: in this sequence, how do we handle hys(final uLow=uLow, uHigh=uHigh)
[ ] How to handle conditions for assignment? eg: [here] (https://github.com/lbl-srg/modelica-json/blob/issue214_cxf/test/FromModelica/RemovableInputs.mo)

mwetter commented 4 months ago

Consider changing TrueHoldWithReset to TrueHold: https://github.com/lbl-srg/modelica-buildings/issues/3689 done

JayHuLBL commented 4 months ago

In the section 7.5 Elementary block:

[ ] Every block should have the icon.
[ ] The icons' color are not consistent. Block like Add exists in both Reals and Integers packages, but the icon color is blue (indicating it is a block in the Reals package).
[ ] In the documentation when there is a line change, it is possible that one word might be cut-off.
[ ] In the description, it should use the font Courier New for the variable names.
[ ] Functions should be reformatted.
[ ] Some hyperlink should be removed, like in the description of the block Reals.Sin.
[ ] There areSwitch blocks in CDL.Logical and CDL.Integers, and also CDL.Reals.
[ ] It's necessary to redraw some figures, as some figures are too big (TrueHoldWithReset), or too small (Latch).
[ ] Differentiate Enthalpy and SpecificEnthalpy.
[ ] OnCounter block should be in the Integers package.
[ ] Blocks in the Sources package should be the sub package of Reals, Integers, and Logical.
[ ] In CDL, there are Buildings.Controls.OBC.CDL.Constants and Buildings.Controls.OBC.CDL.Types. Where should this enumeration and the constants be introduced?

mwetter commented 3 months ago

[ ] For the staging rotation of the heat pump plants, a sort block was needed that also outputs the index. This is implemented in https://github.com/lbl-srg/modelica-buildings/blob/27afa7c54d7b2ee3820ed1b7400c0e8ee3bd5361/Buildings/Templates/Plants/Controls/Utilities/SortWithIndices.mo and similar to CDL.Reals.Sort except for the additional output. We should propose to change the CDL block accordingly. Proposed to be added (email sent to Paul on 4/18/24)
[ ] Make sure CDL.Logical.Latch has documentation updated as in https://github.com/lbl-srg/modelica-buildings/pull/3800/files#diff-6fe286a5bd9201123e0b7af359ef7ac0a0dc5874445f519315dedf9fb191d551R3

anandkp92 commented 2 months ago

[ ] add following specification: ensure that parameters have one of default
```
annotation (__cdl(default=0))
```
or a value binding (parameter k = 0) so that we can evaluate expressions.

lbl-srg / obc