From 080c8069aa7dc59d8c40c777660c975f2d86f572 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Wed, 16 Sep 2026 16:14:34 -0400 Subject: [PATCH 1/6] [wip] Reach a ValueType's value properties, and show the enum defaults MATLAB shows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simulink.ValueType's schema layout already listed min, max and unit, but the node declared neither field. Those three keys resolve through schemaBridge's ATOM_BY_KEY to atoms that read node FIELDS, so each row rendered blank — and toPIObject still counted the key as shown, so the "Other" catch-all suppressed the raw property too. A dictionary carrying "Unit": "m" had no way to display it in either pane. ValueTypeNode now models the surface MATLAB models: Dimensions, Complexity, DimensionsMode, Min, Max, Unit. Unit is read FIRST here and DocUnits second, the opposite order from a Parameter or Signal, because MATLAB serializes a ValueType's unit as Unit and those two as DocUnits; both spellings are still read so a file written either way displays. No *_internal aliasing: a ValueType written with every property non-default came back with flat keys only. An absent Complexity or DimensionsMode means 'real' and 'Fixed', not the 'auto' a Signal defaults to. That default is read on two independent paths — the table column takes it from the schema descriptor, the Property Inspector from the node field — so valueType.json overrides the shared dimensionsMode default per class and a test asserts the two paths agree rather than asserting each alone. Simulink.BusElement had the same pair defaulting to '' and showed blanks where MATLAB shows a value; its write-back guards now compare against the default instead of testing truthiness, which that change would otherwise have made always true. _rejectUnknownEnumeral moves from BusElementNode, its only caller, up to DataNode beside _setMinMax: ValueType's two enums need the same rule, and copying it would have copied the '' -is-a-CLEAR licence, which is exactly the subtlety that rots out of sync between two copies. _normalizeMinMax joins it there, replacing the identical copies ParameterNode and BusElementNode each carried. Also adds the schema descriptors the enum and lookup-table classes need next (isTunableInCode, the three StructTypeInfo keys, SupportTunableSize and the different-breakpoint-sizes flag), all measured off dictionaries MATLAB wrote. --- src/datamodel/node/DataNode.ts | 65 +++ src/datamodel/node/data/BusNode.ts | 87 +--- src/datamodel/node/data/ParameterNode.ts | 7 +- src/datamodel/node/data/ValueTypeNode.ts | 124 ++++- src/datamodel/schema/classes/valueType.json | 4 +- src/datamodel/schema/props/codeGen.json | 7 +- src/datamodel/schema/props/dataObject.json | 2 + test/busElementDisplayDefaults.test.ts | 199 +++++++++ test/busElementEnumEdit.test.ts | 15 +- test/valueTypeValueProps.test.ts | 472 ++++++++++++++++++++ 10 files changed, 897 insertions(+), 85 deletions(-) create mode 100644 test/busElementDisplayDefaults.test.ts create mode 100644 test/valueTypeValueProps.test.ts diff --git a/src/datamodel/node/DataNode.ts b/src/datamodel/node/DataNode.ts index 94d325b..8d1ce21 100644 --- a/src/datamodel/node/DataNode.ts +++ b/src/datamodel/node/DataNode.ts @@ -440,6 +440,71 @@ export default class DataNode extends BaseNode { return true; } + // MATLAB's empty bound is `[]`, and `[]` is truthy: left as it arrives it reaches + // the table as the text `[]` and the writers as a real value, so a class holding a + // bound as a node field normalizes it to `undefined` on the way in — the same + // "no bound" _setMinMax above stores for a cleared cell. Shared rather than + // per-class so the two ends of that round trip cannot disagree about what an + // absent bound is: ParameterNode, BusElementNode and ValueTypeNode all call it, and + // the first two each carried an identical private copy until the third needed one. + static _normalizeMinMax(val: unknown): number | undefined { + if (Array.isArray(val) && val.length === 0) { return undefined; } + return val as number | undefined; + } + + // Refuse a value MATLAB's enum does not contain, for any property surfaced as a + // dropdown (Complexity, DimensionsMode). Without this the edit reaches the generic + // branch for a string field above, which stores whatever text arrived: the table's + // own combobox can only offer legal choices, but the Property Inspector has no + // combobox and seeds a plain text box, so 'Real' or 'fixed' would be written into a + // file MATLAB then refuses to load — the failure an unlock has to rule out before + // it is an unlock at all. + // + // The legal set is read off the prop atom's readOptions — the SAME call the + // cell's dropdown is built from (BaseNode.getPropInfo) — rather than restated + // here. Two copies of an enum is how a UI ends up offering two choices and + // accepting three. + // + // The wording is MATLAB's own, from a probe of the live object (recorded in + // Simulink.BusElement.md): assigning anything else raises "There is no + // enumerated value named 'X'." Note this is MATLAB's message for a rejected + // ASSIGNMENT; that the values we do accept produce a file MATLAB reopens with + // the same values is the live tier's claim to make, and it has not been run + // here (test/parity/matlab/writeback.live.test.ts, gated on DEX_MATLAB_CMD). + // + // Lives here beside _setMinMax, rather than on BusElementNode where it started with + // one caller, because Simulink.ValueType's Complexity/DimensionsMode are the same + // two closed enums and need the same rule. Copying it would have copied the + // ''-is-a-CLEAR licence below, which is precisely the kind of subtlety that rots out + // of sync between two copies. + _rejectUnknownEnumeral(propName: string, stringValue: string): SetPropertyResult | null { + const prop = this._propFor(propName); + if (!prop || prop.editor !== 'select' || !prop.readOptions) { + return null; + } + // The empty string is a CLEAR, not an illegal enumeral — the same licence + // _setMinMax takes for '' and '[]'. It has to be, because it is what a user + // emptying the cell submits, and because DataModel.editProperty captures the + // prior value for UNDO: refusing '' would leave the undo of a clear silently + // unapplied. Storing '' restores absence rather than writing an illegal value, + // and every write-back gate for these props reads '' as absence too, so the + // object goes back out exactly as it came in. + if (stringValue === '') { + return null; + } + const options = prop.readOptions(this); + if (options.length === 0 || options.indexOf(stringValue) >= 0) { + return null; + } + const current = (this as unknown as Record)[prop.nodeProperty || prop.key]; + return { + error: true, + reason: "There is no enumerated value named '" + stringValue + "'.", + invalidValue: stringValue, + validValue: typeof current === 'string' ? current : '', + }; + } + // No structural editing by default. The classes that DO manage children (bus, // enum type, struct, MATLAB array/cell/string) override both, delegating to // childEdit.ts. Returning null here — rather than inheriting a wrapper around diff --git a/src/datamodel/node/data/BusNode.ts b/src/datamodel/node/data/BusNode.ts index ce042a0..ae75c70 100644 --- a/src/datamodel/node/data/BusNode.ts +++ b/src/datamodel/node/data/BusNode.ts @@ -21,11 +21,11 @@ export class BusElementNode extends BaseBusElementNode { // Verified against MATLAB (Simulink.BusElement): these are real element // properties that were not surfaced before, so their columns read empty. // Complexity {real|complex} and DimensionsMode {Fixed|Variable} are closed - // enums and are now EDITABLE selects — validated against the enum below and - // written back in _applyElementOverrides. Dimensions is a positive double - // vector with a symbolic-char alternative and stays read-only: nothing in the - // source claims its constraint was worked out, and an unlock is worth only as - // much as the rule that refuses a bad value. + // enums and are now EDITABLE selects — validated by + // DataNode._rejectUnknownEnumeral and written back in _applyElementOverrides. + // Dimensions is a positive double vector with a symbolic-char alternative and + // stays read-only: nothing in the source claims its constraint was worked out, + // and an unlock is worth only as much as the rule that refuses a bad value. Complexity: string; Dimensions: unknown; DimensionsMode: string; @@ -45,14 +45,16 @@ export class BusElementNode extends BaseBusElementNode { // DataType); an unset type means the Simulink default of 'double'. const rawDataType = props.DataType_internal !== undefined ? props.DataType_internal : props.DataType; this.DataType = (rawDataType as string) || 'double'; - this.Complexity = (props.Complexity as string) || ''; + // MATLAB's own defaults for an element that declares neither (probed on a live + // Simulink.BusElement): Complexity 'real', DimensionsMode 'Fixed' — the same pair + // Simulink.ValueType defaults to, and NOT the 'auto' a Simulink.Signal uses. These + // are DISPLAY values only: the write-back gates in _applyElementOverrides compare + // against the same two literals, so an element the file left silent still saves + // silent. Before this the fallback was '', which showed blanks where MATLAB shows + // a value. + this.Complexity = (props.Complexity as string) || 'real'; this.Dimensions = props.Dimensions; - this.DimensionsMode = (props.DimensionsMode as string) || ''; - } - - static _normalizeMinMax(val: unknown): number | undefined { - if (Array.isArray(val) && val.length === 0) { return undefined; } - return val as number | undefined; + this.DimensionsMode = (props.DimensionsMode as string) || 'Fixed'; } // A StructType's elements use the struct-element icon; a derived @@ -108,54 +110,6 @@ export class BusElementNode extends BaseBusElementNode { return super.setProperty(propName, stringValue); } - // Refuse a value MATLAB's enum does not contain, for either element property - // surfaced as a dropdown (Complexity, DimensionsMode). Without this the edit - // reaches DataNode's generic branch for a string field, which stores whatever - // text arrived: the table's own combobox can only offer legal choices, but the - // Property Inspector has no combobox and seeds a plain text box, so 'Real' or - // 'fixed' would be written into a file MATLAB then refuses to load — the - // failure an unlock has to rule out before it is an unlock at all. - // - // The legal set is read off the prop atom's readOptions — the SAME call the - // cell's dropdown is built from (BaseNode.getPropInfo) — rather than restated - // here. Two copies of an enum is how a UI ends up offering two choices and - // accepting three. - // - // The wording is MATLAB's own, from a probe of the live object (recorded in - // Simulink.BusElement.md): assigning anything else raises "There is no - // enumerated value named 'X'." Note this is MATLAB's message for a rejected - // ASSIGNMENT; that the values we do accept produce a file MATLAB reopens with - // the same values is the live tier's claim to make, and it has not been run - // here (test/parity/matlab/writeback.live.test.ts, gated on DEX_MATLAB_CMD). - _rejectUnknownEnumeral(propName: string, stringValue: string): SetPropertyResult | null { - const prop = this._propFor(propName); - if (!prop || prop.editor !== 'select' || !prop.readOptions) { - return null; - } - // The empty string is a CLEAR, not an illegal enumeral — the same licence - // _setMinMax takes for '' and '[]'. It has to be, because it is what UNDO - // submits: an element that never carried the property reads as '' (see the - // constructor's `|| ''`), DataModel.editProperty captures that as the prior - // value, and refusing it would leave the undo of a perfectly good edit - // silently unapplied. Storing '' restores absence rather than writing an - // illegal value — _applyElementOverrides then omits the key entirely, so the - // element goes back out exactly as it came in. - if (stringValue === '') { - return null; - } - const options = prop.readOptions(this); - if (options.length === 0 || options.indexOf(stringValue) >= 0) { - return null; - } - const current = (this as unknown as Record)[prop.nodeProperty || prop.key]; - return { - error: true, - reason: "There is no enumerated value named '" + stringValue + "'.", - invalidValue: stringValue, - validValue: typeof current === 'string' ? current : '', - }; - } - _applyElementOverrides(props: Record): void { const sp = this.serial._properties as Record; const minKey = 'Min_internal' in sp ? 'Min_internal' : 'Min'; @@ -183,10 +137,15 @@ export class BusElementNode extends BaseBusElementNode { // `

complex

` or as its JSON member); // what was missing was the copy from the node field into the bag, so the // serializer faithfully re-emitted the value parsed from the file. - // Guarded the same way as Description above so an element that never - // carried the key does not gain one on a clean round trip. - if ('Complexity' in sp || this.Complexity) { props.Complexity = this.Complexity; } - if ('DimensionsMode' in sp || this.DimensionsMode) { props.DimensionsMode = this.DimensionsMode; } + // Guarded like DataType above — against the DEFAULT, not on truthiness — so an + // element that never carried the key does not gain one on a clean round trip. + // Truthiness was enough only while an absent enum read as ''; now that it reads as + // MATLAB's default the same test is always true, and every untyped element in every + // dictionary would come back with both keys added. The extra `this.X &&` covers the + // CLEAR: emptying the cell stores '' (see DataNode._rejectUnknownEnumeral), which is + // not a value either property has, so it means absence here too. + if ('Complexity' in sp || (this.Complexity && this.Complexity !== 'real')) { props.Complexity = this.Complexity; } + if ('DimensionsMode' in sp || (this.DimensionsMode && this.DimensionsMode !== 'Fixed')) { props.DimensionsMode = this.DimensionsMode; } if ('Description' in sp || this.Description) { props.Description = this.Description; } } } diff --git a/src/datamodel/node/data/ParameterNode.ts b/src/datamodel/node/data/ParameterNode.ts index 7b9b0b1..996f7f2 100644 --- a/src/datamodel/node/data/ParameterNode.ts +++ b/src/datamodel/node/data/ParameterNode.ts @@ -374,10 +374,9 @@ export default class ParameterNode extends DataNode { return new ParameterNode(name, parent, props, { _rawVal: rawVal, _properties: props } as Record); } - static _normalizeMinMax(val: unknown): number | undefined { - if (Array.isArray(val) && val.length === 0) { return undefined; } - return val as number | undefined; - } + // _normalizeMinMax is inherited from DataNode — this class had its own identical copy + // until Simulink.ValueType became the third class to need it. Static inheritance keeps + // every existing `ParameterNode._normalizeMinMax(...)` call site spelled the same. static parse(rawVal: Record, name: string, parent: BaseNode | null): ParameterNode { const elem = rawVal._elements && (rawVal._elements as unknown[])[0]; diff --git a/src/datamodel/node/data/ValueTypeNode.ts b/src/datamodel/node/data/ValueTypeNode.ts index 94ccfc3..af49aa7 100644 --- a/src/datamodel/node/data/ValueTypeNode.ts +++ b/src/datamodel/node/data/ValueTypeNode.ts @@ -1,15 +1,67 @@ // Copyright 2026 The MathWorks, Inc. import SimulinkObjectNode from '../SimulinkObjectNode.js'; -import type { PropClass } from '../BaseNode.js'; +import type { PropClass, PIGroupDef } from '../BaseNode.js'; import type BaseNode from '../BaseNode.js'; +import type { SetPropertyResult } from '../DataNode.js'; import PropName from '../../prop/PropName.js'; import PropDataType from '../../prop/PropDataType.js'; import PropDescription from '../../prop/PropDescription.js'; +import PropKind from '../../prop/PropKind.js'; +import PropClassAtom from '../../prop/PropClass.js'; +import PropMin from '../../prop/PropMin.js'; +import PropMax from '../../prop/PropMax.js'; +import PropUnit from '../../prop/PropUnit.js'; +import PropComplexity from '../../prop/PropComplexity.js'; +import PropDimensions from '../../prop/PropDimensions.js'; +import PropDimensionsMode from '../../prop/PropDimensionsMode.js'; const CLASS_NAME = 'Simulink.ValueType'; export default class ValueTypeNode extends SimulinkObjectNode { Description: string; DataType: string; - constructor(name: string, parent: BaseNode | null, props: Record, serial: Record) { super(name, parent, serial); this.Description = (props.Description as string) || ''; this.DataType = (props.DataType as string) || 'double'; } + // The rest of the value-property surface MATLAB models on Simulink.ValueType. These + // fields are what made valueType.json's existing layout work: `min`/`max`/`unit` resolve + // through schemaBridge's ATOM_BY_KEY to atoms that read node FIELDS, and the fields did + // not exist — so each row rendered blank AND was added to shownKeys, which had the + // "Other" catch-all suppress the raw key as well. A `"Unit": "m"` in the file was + // unreachable in the UI, in both panes at once (test/parity/artifacts/text/params.sldd's + // MyValueType is exactly that dictionary). Complexity {real|complex} and DimensionsMode + // {Fixed|Variable} are closed enums and EDITABLE selects, validated by + // DataNode._rejectUnknownEnumeral; Dimensions stays read-only for the reason + // BusElementNode records — its constraint has not been worked out, and an unlock is + // worth only as much as the rule that refuses a bad value. + Dimensions: unknown; + Complexity: string; + DimensionsMode: string; + Min: number | undefined; + Max: number | undefined; + Unit: string; + constructor(name: string, parent: BaseNode | null, props: Record, serial: Record) { + super(name, parent, serial); + this.Description = (props.Description as string) || ''; + this.DataType = (props.DataType as string) || 'double'; + this.Dimensions = props.Dimensions; + // MATLAB's defaults for a ValueType that declares neither (probed on a live object): + // 'real' and 'Fixed' — NOT the 'auto' a Simulink.Signal defaults to, which is why + // schema/classes/valueType.json overrides the shared dimensionsMode descriptor's + // default per class rather than changing it there. That JSON default feeds the TABLE + // column (via schemaColumns) while this fallback feeds the Property Inspector, so both + // have to say Fixed; valueTypeValueProps.test.ts asserts the two agree rather than + // asserting each on its own. Display values only: the gates in _serializedOverrides + // compare against the same literals, so a ValueType the file left silent saves silent. + this.Complexity = (props.Complexity as string) || 'real'; + this.DimensionsMode = (props.DimensionsMode as string) || 'Fixed'; + // No `*_internal` alias reading, unlike BusElementNode: a probe wrote a ValueType with + // every property non-default and the dictionary came back with flat keys only. That + // aliasing is specific to bus elements in SLX XML, so looking for it here would be + // inventing a spelling MATLAB does not use. + this.Min = ValueTypeNode._normalizeMinMax(props.Min); + this.Max = ValueTypeNode._normalizeMinMax(props.Max); + // `Unit` FIRST, the opposite order from SignalNode/BusElementNode: MATLAB serializes a + // Simulink.ValueType's unit as `Unit` and a Simulink.Parameter's or Signal's as + // `DocUnits` (both measured off dictionaries MATLAB wrote). Both spellings are read so + // a file written either way displays, but this class's own canonical key wins. + this.Unit = (props.Unit as string) || (props.DocUnits as string) || ''; + } get icon(): string { return this.isDerived ? 'typeSignalUI' : 'wsValue'; } get className(): string { return CLASS_NAME; } // The DataType column shows the ValueType's underlying DataType property @@ -19,13 +71,67 @@ export default class ValueTypeNode extends SimulinkObjectNode { // editable (the DataType is surfaced in the Data Type column). get displayValue(): string { return ''; } get valueEditable(): boolean { return false; } - getProperties(): PropClass[] { return [PropName, PropDataType, PropDescription]; } - // PI layout is schema-driven (schema/classes/valueType.json). - // DataType spells its own gate rather than going through _gatedProps: 'double' is what an - // ABSENT DataType means, and it is also truthy, so the shared truthiness test would write - // that default back into every ValueType a dictionary never declared one for. Compared - // against the default instead, only a type the file stated or the model changed is saved. - _serializedOverrides(): Record { const sp = this.serial._properties as Record; const overrides: Record = {}; if ('DataType' in sp || this.DataType !== 'double') { overrides.DataType = this.DataType; } return Object.assign(overrides, this._gatedProps({ Description: this.Description })); } + getProperties(): PropClass[] { return [PropName, PropDataType, PropDimensions, PropComplexity, PropDimensionsMode, PropMin, PropMax, PropUnit, PropDescription]; } + // Override-driven rather than schema-driven, even though schema/classes/valueType.json + // carries a layout with exactly these groups and this order — the JSON stays, because it + // is what schemaColumns reads for the table's dimensionsMode column, and the two are + // pinned against each other by test/valueTypeValueProps.test.ts. + // + // The reason is that `complexity` and `dimensionsMode` are NOT in schemaBridge's + // ATOM_BY_KEY, so the schema route resolves them to the raw descriptor, whose readValue + // hydrates from `serial._properties`. That is right for a read-only projection and wrong + // the moment the property becomes an editable node field: an edit lands on the field, the + // table re-reads the field and updates, and the PI keeps reading the untouched source bag + // — the same value showing two different things in two panes. (trySetSchemaProperty + // cannot close that gap: both descriptors are `editor: 'label'`, so it declines them and + // nothing writes back into the bag.) Going through the atoms makes both panes read the + // one field. BusElementNode is override-driven for the same reason. + getPILayout(): PIGroupDef[] { + return [ + { group: 'General', items: [PropName, PropDataType, PropKind, PropClassAtom] }, + { group: 'Value Properties', items: [ + PropDimensions, PropComplexity, + PropMin, PropMax, PropUnit, + PropDimensionsMode, PropDescription, + ] }, + ]; + } + // Min/Max take the shared, MATLAB-verified "finite real double scalar" validator rather + // than DataNode's generic numeric path, which wrongly accepts Inf/NaN; the two enums take + // the shared enumeral check, which reads its legal set from the prop atom's readOptions so + // the values accepted here and the values the dropdown offers cannot diverge. + setProperty(propName: string, stringValue: string): true | SetPropertyResult { + if (propName === 'Min' || propName === 'Max') { return this._setMinMax(propName, stringValue); } + const notAnEnumeral = this._rejectUnknownEnumeral(propName, stringValue); + if (notAnEnumeral) { return notAnEnumeral; } + return super.setProperty(propName, stringValue); + } + // Keys in MATLAB's own order (alphabetical, as it writes them), so a ValueType that gains + // a key still reads the way a MATLAB-written one does. Every gate here says the same + // thing: write the key if the FILE carried it, or if the live value is something other + // than what its absence means. DataType and the two enums spell that out against their + // default rather than going through _gatedProps, because each default is truthy and the + // shared truthiness test would write it back into every ValueType a dictionary never + // declared one for; the enums also need `this.X &&` so a CLEAR (which stores '') reads as + // absence and not as a value. Dimensions is deliberately absent: it is read-only, so + // there is no live value to write over the bag both paths already merge the file's own + // keys from. + _serializedOverrides(): Record { + const sp = this.serial._properties as Record; + const unitKey = 'DocUnits' in sp ? 'DocUnits' : 'Unit'; + const overrides: Record = {}; + if ('Complexity' in sp || (this.Complexity && this.Complexity !== 'real')) { overrides.Complexity = this.Complexity; } + if ('DataType' in sp || this.DataType !== 'double') { overrides.DataType = this.DataType; } + Object.assign(overrides, this._gatedProps({ Description: this.Description })); + if ('DimensionsMode' in sp || (this.DimensionsMode && this.DimensionsMode !== 'Fixed')) { overrides.DimensionsMode = this.DimensionsMode; } + // A cleared bound goes out as `[]` — MATLAB's own empty — and not as the value the + // file held there, so emptying the Minimum box does not silently save the old number + // back. See BusElementNode._applyElementOverrides for the full note. + if ('Max' in sp || this.Max !== undefined) { overrides.Max = this.Max !== undefined ? this.Max : []; } + if ('Min' in sp || this.Min !== undefined) { overrides.Min = this.Min !== undefined ? this.Min : []; } + if (unitKey in sp || this.Unit) { overrides[unitKey] = this.Unit; } + return overrides; + } static get defaultName(): string { return 'ValueType'; } static createDefault(name: string, parent: BaseNode | null): ValueTypeNode { const rawVal = ValueTypeNode._defaultRawVal(CLASS_NAME); const props = ValueTypeNode._propsOf(rawVal); return new ValueTypeNode(name, parent, props, { _rawVal: rawVal, _properties: props } as Record); } static parse(rawVal: Record, name: string, parent: BaseNode | null): ValueTypeNode { const props = ValueTypeNode._propsOf(rawVal); return new ValueTypeNode(name, parent, props, { _rawVal: rawVal, _properties: props } as Record); } diff --git a/src/datamodel/schema/classes/valueType.json b/src/datamodel/schema/classes/valueType.json index fa9c41d..c9a7f69 100644 --- a/src/datamodel/schema/classes/valueType.json +++ b/src/datamodel/schema/classes/valueType.json @@ -4,11 +4,11 @@ "dataType", "description", "dimensions", - "dimensionsMode", + { "$ref": "dimensionsMode", "default": "Fixed" }, "complexity", "min", "max", - "unit" + { "$ref": "unit", "sourcePath": "Unit" } ], "layout": [ { "group": "General", "items": ["name", "dataType", "kind", "class"] }, diff --git a/src/datamodel/schema/props/codeGen.json b/src/datamodel/schema/props/codeGen.json index 71ffc55..d760011 100644 --- a/src/datamodel/schema/props/codeGen.json +++ b/src/datamodel/schema/props/codeGen.json @@ -14,5 +14,10 @@ "setFunction": { "label": "Set Function", "sourcePath": "CoderInfo.CustomAttributes.SetFunction", "type": "string", "editor": "label", "default": "" }, "preserveElementDimensions": { "label": "Preserve Element Dimensions", "sourcePath": "PreserveElementDimensions", "type": "bool", "editor": "label", "default": "" }, - "addClassNameToEnumNames": { "label": "Add Class Name To Enum Names", "sourcePath": "AddClassNameToEnumNames", "type": "bool", "editor": "label", "default": "" } + "addClassNameToEnumNames": { "label": "Add Class Name To Enum Names", "sourcePath": "AddClassNameToEnumNames", "type": "bool", "editor": "label", "default": "" }, + "isTunableInCode": { "label": "Is Tunable In Code", "sourcePath": "IsTunableInCode", "type": "bool", "editor": "label", "default": "" }, + + "structTypeName": { "label": "Name", "sourcePath": "StructTypeInfo.Name", "type": "string", "editor": "label", "default": "" }, + "structTypeDataScope": { "label": "Data Scope", "sourcePath": "StructTypeInfo.DataScope", "type": "string", "editor": "label", "default": "Auto" }, + "structTypeHeaderFile": { "label": "Header File", "sourcePath": "StructTypeInfo.HeaderFileName", "type": "string", "editor": "label", "default": "" } } diff --git a/src/datamodel/schema/props/dataObject.json b/src/datamodel/schema/props/dataObject.json index e27c9ae..0372662 100644 --- a/src/datamodel/schema/props/dataObject.json +++ b/src/datamodel/schema/props/dataObject.json @@ -11,5 +11,7 @@ "sampleTime": { "label": "Sample Time", "sourcePath": "SampleTime", "type": "any", "editor": "label", "default": "" }, "samplingMode": { "label": "Sampling Mode", "sourcePath": "SamplingMode", "type": "string", "editor": "label", "default": "" }, "breakpointsSpecification": { "label": "Breakpoints Specification", "sourcePath": "BreakpointsSpecification", "type": "string", "editor": "label", "default": "Explicit values" }, + "supportTunableSize": { "label": "Support Tunable Size", "sourcePath": "SupportTunableSize", "type": "bool", "editor": "label", "default": "" }, + "allowDifferentTableBpSizes": { "label": "Allow Multiple Instances Of Type To Have Different Table Breakpoint Sizes", "sourcePath": "AllowMultipleInstancesOfTypeToHaveDifferentTableBreakpointSizes", "type": "bool", "editor": "label", "default": "" }, "bank": { "label": "Bank", "sourcePath": "Bank", "type": "any", "editor": "label", "default": "" } } diff --git a/test/busElementDisplayDefaults.test.ts b/test/busElementDisplayDefaults.test.ts new file mode 100644 index 0000000..74e1d24 --- /dev/null +++ b/test/busElementDisplayDefaults.test.ts @@ -0,0 +1,199 @@ +// Copyright 2026 The MathWorks, Inc. +// +// A bus element that declares neither Complexity nor DimensionsMode now DISPLAYS MATLAB's +// default for it — 'real' and 'Fixed' — where it used to display a blank. MATLAB shows a +// value for those properties on every element it has, because the object always has one; +// only the FILE is silent. +// +// The whole risk in that change is on the save side, and it is the reason this file exists +// separately from busElementEnumEdit.test.ts. The write-back gates read the same node +// fields the display does, and they were `if ('Complexity' in sp || this.Complexity)` — a +// truthiness test that is correct only while an absent property reads as ''. Give the field +// a non-empty default and that test is ALWAYS true, so every untyped element in every +// dictionary would silently gain `Complexity: "real"` and `DimensionsMode: "Fixed"` on the +// next save. Open a file, change nothing, save, get a diff on every element: the failure is +// in a file the user never edited, which is the worst place for it. +// +// So the gates now compare against the default, exactly as DataType's already did, plus a +// `this.X &&` for the CLEAR — emptying the cell stores '' (DataNode._rejectUnknownEnumeral +// keeps '' as a clear rather than refusing it as an illegal enumeral), and '' is not a value +// either enum has, so it means absence here too. +// +// The invariant, stated so it is testable, and it is a BETWEEN-paths one rather than a fact +// about either: what an element DISPLAYS and what it SAVES are now allowed to differ, and +// the display value MATLAB supplies for an absent property must never become a saved key. + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { createSession } from '../src/index.js'; +import { + loadModel, + entryByName, + serializeModel, + reparseEntry, + type SlddFormat, +} from './parity/fidelity/roundTripHarness.js'; + +// arch.sldd's DataInterface bus, whose three elements are exactly the three cases needed: +// Element (Complexity 'real', DimensionsMode 'Fixed' — both declared), Element1 ('complex' / +// 'Variable'), and `a`, whose property bag is just { Name: 'a' } — an element that carries +// neither key, which is also what a freshly added element looks like. +function archBusSession() { + const path = fileURLToPath(new URL('./fixtures/arch.sldd', import.meta.url)); + const s = createSession(); + const src = s.addDataSource('arch.sldd', JSON.parse(readFileSync(path, 'utf8'))) as any; + const bus = (src.flatten() as any[]).find((n) => n.name === 'DataInterface' && n.className === 'Simulink.Bus'); + if (!bus) throw new Error('no DataInterface bus in arch.sldd'); + s.setActiveContext(src); + return { s, src, bus }; +} + +const elementNamed = (bus: any, name: string) => bus.children.find((c: any) => c.name === name); +// A bus element has ONE serialization method, and both of the parent bus's save paths call +// it (BaseBusNode._getSerializedProperties maps it over the children; its serializeValue +// wraps that) — so this single bag is what reaches the file in either format. +const savedBag = (elem: any) => (elem.serializeValue() as { _properties: Record })._properties; +const piValue = (node: any, key: string) => (node.toPIObject().objects[0] as Record)[key]; +const cellText = (node: any, column: string): string => { + const cell = node.toRow()[column]; + return typeof cell === 'string' ? cell : (cell as { text: string }).text; +}; + +const ENUMS = [ + { prop: 'complexity', field: 'Complexity', dflt: 'real', other: 'complex' }, + { prop: 'dimensionsMode', field: 'DimensionsMode', dflt: 'Fixed', other: 'Variable' }, +] as const; + +describe('a bus element that declares neither enum property', () => { + for (const e of ENUMS) { + it(`displays MATLAB's ${e.field} default in the table and the PI`, () => { + const { bus } = archBusSession(); + const elem = elementNamed(bus, 'a'); + expect(elem[e.field]).toBe(e.dflt); + expect(cellText(elem, e.prop)).toBe(e.dflt); + expect(piValue(elem, e.prop)).toBe(e.dflt); + }); + + it(`does not gain a ${e.field} key on save`, () => { + // The gate's reach: the display default is not a value the user set, so it is not a + // value the file learns about. + const { bus } = archBusSession(); + expect(savedBag(elementNamed(bus, 'a'))).toEqual({ Name: 'a' }); + }); + + it(`does not gain a ${e.field} key when the user picks that same default`, () => { + // Choosing 'real' on an element that was already showing 'real' changes nothing the + // file has to record. This is the case the `!== default` half of the gate handles. + const { bus } = archBusSession(); + const elem = elementNamed(bus, 'a'); + expect(elem.setProperty(e.prop, e.dflt)).toBe(true); + expect(savedBag(elem)).toEqual({ Name: 'a' }); + }); + + it(`does not gain a ${e.field} key when an edit is cleared again`, () => { + // A clear stores '', which the `this.X &&` half reads as absence. Without it the save + // writes an empty char where MATLAB expects an enumeral. + const { bus } = archBusSession(); + const elem = elementNamed(bus, 'a'); + expect(elem.setProperty(e.prop, e.other)).toBe(true); + expect(elem.setProperty(e.prop, '')).toBe(true); + expect(savedBag(elem)).toEqual({ Name: 'a' }); + }); + + it(`still saves a ${e.field} the user really changed`, () => { + // The other direction the gate can fail in: over-tighten it and an edit made in the + // UI is dropped on save, surviving only until the file is reopened. + const { bus } = archBusSession(); + const elem = elementNamed(bus, 'a'); + expect(elem.setProperty(e.prop, e.other)).toBe(true); + expect(savedBag(elem)[e.field]).toBe(e.other); + }); + } +}); + +describe('a bus element that DID declare the enum properties', () => { + it('shows its own values, not the defaults', () => { + // A default that overrode a declared value would be the same defect from the other + // side, and Element1 differs from the default on both properties at once. + const { bus } = archBusSession(); + expect(elementNamed(bus, 'Element').Complexity).toBe('real'); + expect(elementNamed(bus, 'Element').DimensionsMode).toBe('Fixed'); + expect(elementNamed(bus, 'Element1').Complexity).toBe('complex'); + expect(elementNamed(bus, 'Element1').DimensionsMode).toBe('Variable'); + }); + + for (const e of ENUMS) { + it(`keeps a declared ${e.field} on save even when its value IS the default`, () => { + // The `key in sp` half: a key MATLAB wrote must survive a save whatever its value, or + // opening a dictionary and saving it produces a diff — and Element declares both + // properties AT their defaults, which is exactly the case a default-only gate loses. + const { bus } = archBusSession(); + const elem = elementNamed(bus, 'Element'); + expect((elem.serial._properties as Record)[e.field]).toBe(e.dflt); + expect(savedBag(elem)[e.field]).toBe(e.dflt); + }); + + it(`saves a declared ${e.field} the user set back to the default`, () => { + // Element1 declares the non-default; setting it to the default must be written, not + // dropped as "same as absent" — the file said something about this property, so the + // saved file has to say the new thing. + const { bus } = archBusSession(); + const elem = elementNamed(bus, 'Element1'); + expect(elem.setProperty(e.prop, e.dflt)).toBe(true); + expect(savedBag(elem)[e.field]).toBe(e.dflt); + }); + } +}); + +describe('the bus as a whole', () => { + it('adds no element key to a bus nobody edited', () => { + // Stated over the parent, because that is the object with the save paths: the elements + // reach a file only inside Elements_internal, and this is the "open, save, no diff" + // claim at the level a user would see it. + const { bus } = archBusSession(); + for (const elem of bus.children) { + const arrived = new Set(Object.keys(elem.serial._properties as Record)); + // Name is always written (BaseBusElementNode.serializeValue), so it is the one key an + // element may gain — every fixture element already carries it. + arrived.add('Name'); + expect(Object.keys(savedBag(elem)).sort()).toEqual([...arrived].sort()); + } + }); +}); + +for (const format of ['json', 'binary'] as SlddFormat[]) { + describe(`bus element display defaults — .sldd round trip (${format})`, () => { + // Through a real MATLAB-written dictionary and both writers, because the defaults are + // read on the way IN and the gates fire on the way OUT: a re-parse is the only thing + // that shows the two composing rather than each being right alone. + function freshMyBus(tag: string) { + const uri = `test://buselem-defaults-${format}-${tag}.sldd`; + const model = loadModel(format, 'params.sldd', uri); + return { model, entry: entryByName(model, uri, 'MyBus') }; + } + + it('an untouched bus round-trips with each element keeping exactly its own keys', () => { + const { model, entry } = freshMyBus('clean'); + const before = entry.children.map((c: any) => Object.keys(c.serial._properties as Record).sort()); + + const fresh = reparseEntry(serializeModel(model, format), format, 'params.sldd', 'MyBus'); + const after = fresh.children.map((c: any) => Object.keys(c.serial._properties as Record).sort()); + expect(after).toEqual(before); + }); + + it('an edited element enum survives the round trip while its siblings gain nothing', () => { + const { model, entry } = freshMyBus('edit'); + const target = entry.children[0]; + const siblingKeysBefore = entry.children + .slice(1) + .map((c: any) => Object.keys(c.serial._properties as Record).sort()); + expect(target.setProperty('complexity', 'complex')).toBe(true); + + const fresh = reparseEntry(serializeModel(model, format), format, 'params.sldd', 'MyBus'); + expect(fresh.children[0].Complexity).toBe('complex'); + expect(fresh.children.slice(1).map((c: any) => Object.keys(c.serial._properties as Record).sort())) + .toEqual(siblingKeysBefore); + }); + }); +} diff --git a/test/busElementEnumEdit.test.ts b/test/busElementEnumEdit.test.ts index 60a3f9b..06097c0 100644 --- a/test/busElementEnumEdit.test.ts +++ b/test/busElementEnumEdit.test.ts @@ -173,11 +173,15 @@ describe('bus element enum props — editing through the session', () => { }); it(`an empty ${e.field} clears the property instead of being refused`, () => { - // The one non-enumeral value that must get through, because it is what undo of - // an edit to an element that never carried the property submits. + // The one non-enumeral value that must get through: it is what emptying the cell + // submits, and the write-back guard below reads it as absence rather than as a + // value. Element `a` carries neither key, and now DISPLAYS MATLAB's default for + // it rather than a blank — the display-defaults change. That is why '' is no + // longer what undo of an edit here submits (undo would submit 'real'/'Fixed', + // both legal), but it is still what a cleared cell submits, so the licence stays. const { bus } = archBusSession(); const elem = elementNamed(bus, 'a'); - expect(elem[e.field]).toBe(''); + expect(elem[e.field]).toBe(e.from); expect(elem.setProperty(e.prop, e.to)).toBe(true); expect(elem[e.field]).toBe(e.to); @@ -186,8 +190,9 @@ describe('bus element enum props — editing through the session', () => { }); it(`an element that never carried ${e.field} does not gain the key on save`, () => { - // The write-back guard is `'' in sp || this.`, so an untouched - // element — or one edited and then cleared — must serialize without the key. + // The write-back guard is `'' in sp || (this. && this. !== + // )`, so an untouched element — or one edited and then cleared — must + // serialize without the key. // A phantom key is a spurious diff on every save of a file the user only // opened, and for these two it would also be an empty char where MATLAB // expects an enumeral. diff --git a/test/valueTypeValueProps.test.ts b/test/valueTypeValueProps.test.ts new file mode 100644 index 0000000..1ea50ae --- /dev/null +++ b/test/valueTypeValueProps.test.ts @@ -0,0 +1,472 @@ +// Copyright 2026 The MathWorks, Inc. +// +// A Simulink.ValueType's value properties — Dimensions, Complexity, DimensionsMode, Min, +// Max, Unit — and the fact that they are now reachable at all. +// +// The defect this file locks out was not a missing layout entry. `valueType.json` already +// listed `min`, `max` and `unit`, and schemaBridge resolves those three keys through +// ATOM_BY_KEY to PropMin/PropMax/PropUnit, which read the NODE FIELDS `Min`/`Max`/`Unit`. +// ValueTypeNode declared none of them. So the row rendered blank — and, worse, toPIObject +// still added each key to `shownKeys`, which had the "Other" catch-all suppress the raw +// property too. A `"Unit": "m"` sitting in the file was invisible in BOTH panes at once, +// with nothing anywhere reporting an error. `test/parity/artifacts/text/params.sldd`'s +// MyValueType is exactly that dictionary, and it is asserted below. +// +// The shape of that bug is why the tests here read the SAME value through two surfaces and +// compare them to each other rather than each to a literal: +// +// * the table cell (getProperties → toRow) and the PI row (getPILayout → toPIObject) must +// show one value, because they are now fed by one node field; +// * an ABSENT property's default is read on two INDEPENDENT paths — the table column for +// `dimensionsMode` comes from the schema descriptor's `default` (schemaColumns), the PI +// from the node field's own fallback — and those two are separately authored, in a JSON +// file and in a constructor. `Fixed` in one and `auto` in the other is a live defect +// that no assertion against a literal in only one of them would catch. +// +// MATLAB-measured facts relied on below (probed on R2027a, recorded in the design spec's +// evidence table): a ValueType serializes its unit as `Unit` where a Parameter/Signal uses +// `DocUnits`; an undeclared ValueType is `DataType='double'`, `Complexity='real'`, +// `DimensionsMode='Fixed'` — Fixed, NOT the `auto` a Simulink.Signal defaults to; the +// enums are exactly {real, complex} and {Fixed, Variable}; and a refused assignment is +// reported as "There is no enumerated value named 'X'.". +// +// NOT covered here: that MATLAB reopens a file we wrote and reads these values back. That +// is the live tier (test/parity/matlab/, gated on DEX_MATLAB_CMD) and it has not been run +// against this change. Everything below is in-process. + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { createSession } from '../src/index.js'; +import ValueTypeNode from '../src/datamodel/node/data/ValueTypeNode.js'; +import { schemaColumns } from '../src/datamodel/node/schemaBridge.js'; +import { getSchema, hydrate } from '../src/datamodel/schema/index.js'; +import { + loadModel, + entryByName, + serializeModel, + reparseEntry, + type SlddFormat, +} from './parity/fidelity/roundTripHarness.js'; +import '../src/datamodel/node/data/NodeClassMap.js'; + +// A ValueType built straight from a property bag, which is how the parsers hand one over. +function valueType(properties: Record): any { + const rawVal = { + _array_class: 'Simulink.ValueType', + _array_type: 'MATLABArray', + _dimensions: [1, 1], + _mw_element_type: 'MATLABArray', + _elements: [{ _properties: properties }], + }; + return ValueTypeNode.parse(rawVal, 'V', null); +} + +// The three surfaces under test, each reduced to what a caller actually sees. +const piValue = (node: any, key: string) => (node.toPIObject().objects[0] as Record)[key]; +const piRowKeys = (node: any, groupTitle: string): string[] => { + const g = (node.toPIObject().propertySheet.groups as any[]).find((x) => x.displayName === groupTitle); + return g ? g.items.map((it: any) => it.name) : []; +}; +const piGroups = (node: any): string[] => (node.toPIObject().propertySheet.groups as any[]).map((g) => g.displayName); +// An editable generic column arrives as {text, editable, editor, options}; a read-only one +// as a plain string. Both are "what the cell shows", so this collapses the difference. +const cellText = (node: any, column: string): string => { + const cell = node.toRow()[column]; + return typeof cell === 'string' ? cell : (cell as { text: string }).text; +}; + +// The two save paths, each reduced to the property bag it puts in the file — the same +// reduction test/absentPropertyWriteBack.test.ts uses, for the same reason: this is a +// question about a KEY SET, not about bytes. +const binaryPath = (node: any) => node._getSerializedProperties() as Record; +const textFileBag = (node: any) => { + const sv = node.serializeValue() as { _elements: { _properties: Record }[] }; + return JSON.parse(JSON.stringify({ p: sv._elements[0]._properties })).p as Record; +}; + +// Every property key a MATLAB-written ValueType carries (probe4 wrote one with all of them +// non-default and got these eight, flat — no `*_internal` aliases, unlike a bus element). +const FULL_BAG = { + Complexity: 'complex', + DataType: 'uint16', + Description: 'a speed', + Dimensions: [1, 3], + DimensionsMode: 'Variable', + Max: 100, + Min: 0, + Unit: 'm', +}; + +describe("a ValueType's value properties are reachable", () => { + it('shows Min, Max and Unit the source carried — the rows that used to render blank', () => { + // The exact reproduction: before ValueTypeNode declared these fields, all three of + // these read '' in the PI while the file plainly held the values. + const n = valueType({ Unit: 'm', Min: 0, Max: 100 }); + expect(piValue(n, 'Min')).toBe('0'); + expect(piValue(n, 'Max')).toBe('100'); + expect(piValue(n, 'Unit')).toBe('m'); + + // Min '0' and not '' is the case worth spelling out: a falsy bound is exactly what a + // `|| ''` fallback or a truthiness gate silently loses. + expect(n.Min).toBe(0); + expect(n.Max).toBe(100); + }); + + it('shows Dimensions, Complexity and DimensionsMode the source carried', () => { + const n = valueType(FULL_BAG); + expect(piValue(n, 'dimensions')).toBe('[1 3]'); + expect(piValue(n, 'complexity')).toBe('complex'); + expect(piValue(n, 'dimensionsMode')).toBe('Variable'); + expect(piValue(n, 'DataType')).toBe('uint16'); + expect(piValue(n, 'Description')).toBe('a speed'); + }); + + it('gives the table the same nine columns a bus element has, reading the same fields', () => { + // getProperties drives the table; getPILayout drives the PI. Two lists over one set of + // node fields, so the failure to rule out is a value that appears in one pane only. + const n = valueType(FULL_BAG); + expect(n.getProperties().map((p: any) => p.key)).toEqual([ + 'Name', 'DataType', 'dimensions', 'complexity', 'dimensionsMode', 'Min', 'Max', 'Unit', 'Description', + ]); + for (const [column, key] of [ + ['DataType', 'DataType'], ['dimensions', 'dimensions'], ['complexity', 'complexity'], + ['dimensionsMode', 'dimensionsMode'], ['Min', 'Min'], ['Max', 'Max'], ['Unit', 'Unit'], + ] as const) { + expect(cellText(n, column)).toBe(piValue(n, key)); + } + }); + + it('keeps the PI groups and order the schema layout authored', () => { + // getPILayout is a node override rather than the schema route (see the comment on it: + // the two enums are editable node fields, and the schema route would read them from + // the untouched source bag, so the table and the PI would disagree after an edit). + // valueType.json's layout stays the authored record of the order, so the override has + // to reproduce it — this is what stops the two drifting apart unnoticed. + const n = valueType(FULL_BAG); + expect(piGroups(n)).toEqual(['General', 'Value Properties']); + expect(piRowKeys(n, 'General')).toEqual(['Name', 'DataType', 'Kind', 'Class']); + expect(piRowKeys(n, 'Value Properties')).toEqual([ + 'dimensions', 'complexity', 'Min', 'Max', 'Unit', 'dimensionsMode', 'Description', + ]); + }); + + it('leaves no "Other" group for a ValueType whose every property is modeled', () => { + // The second half of the original defect: a key the layout claims to show is added to + // `shownKeys` whether or not the row displayed anything, so an unreadable row also + // hides the raw value from the catch-all. With all eight keys genuinely shown, "Other" + // must be empty — and if a future key is added to the file format and not to the node, + // this is what surfaces it instead of silently swallowing it. + expect(piGroups(valueType(FULL_BAG))).toEqual(['General', 'Value Properties']); + }); + + it('reads a unit under either spelling, and re-lists neither in "Other"', () => { + // MATLAB writes a ValueType's unit as `Unit`; a Parameter's and a Signal's as + // `DocUnits`. Both are read so a file written either way displays, and PropUnit + // declares both in sourceKeys so the one the file used is not ALSO rendered as a raw + // "Other" row — a duplicate that would show the same unit twice. + const canonical = valueType({ Unit: 'm' }); + expect(piValue(canonical, 'Unit')).toBe('m'); + expect(piGroups(canonical)).toEqual(['General', 'Value Properties']); + + const alternate = valueType({ DocUnits: 'm/s' }); + expect(piValue(alternate, 'Unit')).toBe('m/s'); + expect(piGroups(alternate)).toEqual(['General', 'Value Properties']); + + // `Unit` wins when a file somehow carries both, because it is this class's own key. + expect(valueType({ Unit: 'm', DocUnits: 'm/s' }).Unit).toBe('m'); + }); + + it("the schema's own `unit` descriptor points at Unit for a ValueType and DocUnits elsewhere", () => { + // The node reads both spellings, so nothing above would notice if the schema's + // per-class `sourcePath` override went missing — and the schema is where the asymmetry + // is DECLARED, for any consumer that reads a unit through the descriptor rather than + // through the atom (a class that goes back to the schema PI route, or a `unit` that + // becomes `projected`). Pinned here so the override is a fact with a test behind it + // rather than an unobserved line of JSON. + const unitFor = (cls: string) => getSchema(cls)!.find((p) => p.key === 'unit')!; + expect(unitFor('Simulink.ValueType').sourcePath).toBe('Unit'); + expect(hydrate({ Unit: 'm' }, unitFor('Simulink.ValueType'))).toBe('m'); + // And the shared descriptor is untouched, so Parameter and Signal still read the key + // MATLAB writes for THEM — overriding it in place would have broken both. + expect(unitFor('Simulink.Parameter').sourcePath).toBe('DocUnits'); + expect(unitFor('Simulink.Signal').sourcePath).toBe('DocUnits'); + }); + + it("shows params.sldd's MyValueType unit, in both formats", () => { + // The committed MATLAB-written fixture that demonstrated the bug. The text flavour + // carries `{ Unit: 'm' }` alone; the binary flavour carries all eight keys, with + // `Min`/`Max` written as MATLAB's empty (`Dimension="0*0"`, parsed to `[]`). + for (const format of ['json', 'binary'] as SlddFormat[]) { + const uri = `test://vt-unit-${format}.sldd`; + const vt = entryByName(loadModel(format, 'params.sldd', uri), uri, 'MyValueType'); + expect(vt.Unit).toBe('m'); + expect(piValue(vt, 'Unit')).toBe('m'); + // `[]` is MATLAB's empty bound and it is TRUTHY, so left unnormalized it reaches the + // cell as the text `[]` — a minimum of nothing displayed as a value. + expect(vt.Min).toBeUndefined(); + expect(cellText(vt, 'Min')).toBe(''); + } + }); +}); + +describe("an absent property's default, on both paths at once", () => { + // The table column and the PI read an absent property's default from two separately + // authored places, and the whole point of these assertions is that they are compared to + // EACH OTHER. `dimensionsMode` is the live case: it is `projected: true`, so the shared + // descriptor's default feeds the table, and that default is 'auto' — Simulink.Signal's + // value, not a ValueType's. valueType.json overrides it to 'Fixed' per class; the node + // constructor's fallback is the PI's copy of the same fact. + const schemaDefault = (key: string): string => { + const col = schemaColumns('Simulink.ValueType').find((c) => c.key === key)!; + return col.readValue!(valueType({})); + }; + + for (const [key, expected] of [['dimensionsMode', 'Fixed'], ['complexity', 'real']] as const) { + it(`${key} reads ${expected} from the schema descriptor AND from the node field`, () => { + const n = valueType({}); + expect(schemaDefault(key)).toBe(piValue(n, key)); + // Both, and not just their agreement: two paths that agree on the wrong value are + // still wrong, and MATLAB is the arbiter of which value it is. + expect(schemaDefault(key)).toBe(expected); + }); + } + + it('an undeclared ValueType displays MATLAB defaults across every value property', () => { + const n = valueType({}); + expect(n.DataType).toBe('double'); + expect(n.Complexity).toBe('real'); + expect(n.DimensionsMode).toBe('Fixed'); + expect(cellText(n, 'DataType')).toBe('double'); + expect(cellText(n, 'complexity')).toBe('real'); + expect(cellText(n, 'dimensionsMode')).toBe('Fixed'); + // Dimensions has no default MATLAB writes, so it stays blank rather than inventing 1. + expect(cellText(n, 'dimensions')).toBe(''); + expect(cellText(n, 'Min')).toBe(''); + expect(cellText(n, 'Unit')).toBe(''); + }); +}); + +describe('editing a ValueType value property', () => { + const ENUMS = [ + { prop: 'complexity', field: 'Complexity', options: ['real', 'complex'], from: 'real', to: 'complex', illegal: 'Real' }, + { prop: 'dimensionsMode', field: 'DimensionsMode', options: ['Fixed', 'Variable'], from: 'Fixed', to: 'Variable', illegal: 'fixed' }, + ] as const; + + for (const e of ENUMS) { + it(`${e.field} is an editable select carrying MATLAB's enum`, () => { + const n = valueType({}); + const info = n.getPropInfo(n.getProperties().find((p: any) => p.key === e.prop)); + expect(info.editable).toBe(true); + expect(info.editor).toBe('select'); + expect(info.options).toEqual(e.options); + }); + + it(`a ${e.field} outside the enum is refused, in MATLAB's own wording`, () => { + // The refusal matters most for the Property Inspector, which has no combobox and + // seeds a plain text box: without it 'Real' or 'fixed' would be stored and written + // into a file MATLAB then declines to load, which is invisible from inside here. + const n = valueType({}); + expect(n.setProperty(e.prop, e.illegal)).toEqual({ + error: true, + reason: `There is no enumerated value named '${e.illegal}'.`, + invalidValue: e.illegal, + // The value to restore is the one being DISPLAYED, which for an undeclared + // property is now MATLAB's default rather than a blank. + validValue: e.from, + }); + expect(n[e.field]).toBe(e.from); + }); + + it(`an empty ${e.field} clears the property instead of being refused`, () => { + // '' is not a value either enum has, and it is what emptying the cell submits. It + // has to get through, because the write-back gate reads it as absence — refusing it + // would leave the user unable to undo a value back off a file that never had one. + const n = valueType({}); + expect(n.setProperty(e.prop, e.to)).toBe(true); + expect(n[e.field]).toBe(e.to); + expect(n.setProperty(e.prop, '')).toBe(true); + expect(n[e.field]).toBe(''); + }); + + it(`a legal ${e.field} reaches BOTH the table cell and the PI row`, () => { + // The reason getPILayout is an override: through the schema route these two keys + // resolve to a descriptor that reads the untouched source bag, so an edit would move + // the table cell and leave the PI showing the old value. + const n = valueType({}); + expect(n.setProperty(e.prop, e.to)).toBe(true); + expect(cellText(n, e.prop)).toBe(e.to); + expect(piValue(n, e.prop)).toBe(e.to); + }); + } + + it('Min/Max take the MATLAB-verified finite-real-scalar rule, not the generic numeric path', () => { + // DataNode's generic numeric branch accepts Inf and NaN; MATLAB's + // Simulink.DataObject/setPropValue does not. Routing through _setMinMax is what makes + // a ValueType bound obey the same constraint a Signal's does. + const n = valueType({}); + expect(n.setProperty('Min', '5')).toBe(true); + expect(n.Min).toBe(5); + expect(n.setProperty('Max', 'Inf')).toEqual({ + error: true, + reason: 'Maximum must be a finite real double scalar value', + invalidValue: 'Inf', + validValue: '[]', + }); + expect(n.Max).toBeUndefined(); + // '' and '[]' both clear, MATLAB's own empty. + expect(n.setProperty('Min', '')).toBe(true); + expect(n.Min).toBeUndefined(); + }); + + it('Unit stays read-only, for the reason PropUnit records', () => { + // Simulink parses Unit through a unit-expression parser we cannot replicate, so it is + // surfaced as a label. Asserted so making it editable is a deliberate act. + const n = valueType({ Unit: 'm' }); + expect(n.getPropInfo(n.getProperties().find((p: any) => p.key === 'Unit')).editable).toBe(false); + }); + + it('undo of an edit puts the displayed value back, through the session', () => { + // Undo resubmits the PRIOR value through setProperty, so it meets the same validator + // the edit did. vtSpeed declares Complexity but not DimensionsMode, which covers both + // cases in one node: undoing the declared one restores the file's value, undoing the + // undeclared one restores MATLAB's default — and both have to be values the validator + // accepts, or the undo reports success and applies nothing. + const path = fileURLToPath(new URL('./fixtures/typeLink.sldd', import.meta.url)); + const s = createSession(); + const src = s.addDataSource('typeLink.sldd', JSON.parse(readFileSync(path, 'utf8'))) as any; + const vt = (src.flatten() as any[]).find((n) => n.name === 'vtSpeed' && n.className === 'Simulink.ValueType'); + s.setActiveContext(src); + s.setActive(src, vt); + + expect(s.editProperty(vt.id, 'complexity', 'complex')).toBe(true); + expect(s.editProperty(vt.id, 'dimensionsMode', 'Variable')).toBe(true); + s.undo(); + expect(vt.DimensionsMode).toBe('Fixed'); + s.undo(); + expect(vt.Complexity).toBe('real'); + }); +}); + +describe('what a saved ValueType carries', () => { + it('invents no key for a ValueType the file declared nothing about, on either path', () => { + // The gate's whole purpose: open a dictionary, save it with no edits, get no diff. All + // three defaults here are TRUTHY ('double', 'real', 'Fixed'), so a truthiness gate + // would write every one of them into a file that never had them. + const n = valueType({}); + expect(binaryPath(n)).toEqual({}); + expect(textFileBag(n)).toEqual({}); + }); + + it('writes every key the file declared straight back, on either path', () => { + const n = valueType(FULL_BAG); + expect(binaryPath(n)).toEqual(FULL_BAG); + expect(textFileBag(n)).toEqual(FULL_BAG); + }); + + it('names the same keys on both paths, whatever the source held', () => { + // One list, two writers (SimulinkObjectNode's reason for existing). A key written into + // the binary file and missing from the text one, from the same model in the same + // session, is the hardest kind of difference to notice — either file looks right alone. + for (const bag of [{}, FULL_BAG, { Unit: 'm' }, { DocUnits: 'm/s' }, { Min: 0 }]) { + const n = valueType({ ...bag }); + expect(Object.keys(binaryPath(n)).sort()).toEqual(Object.keys(textFileBag(n)).sort()); + } + }); + + it('persists an edit to each value property, under the key MATLAB uses', () => { + const n = valueType({}); + expect(n.setProperty('complexity', 'complex')).toBe(true); + expect(n.setProperty('dimensionsMode', 'Variable')).toBe(true); + expect(n.setProperty('Min', '0')).toBe(true); + expect(n.setProperty('Max', '100')).toBe(true); + expect(binaryPath(n)).toEqual({ Complexity: 'complex', DimensionsMode: 'Variable', Max: 100, Min: 0 }); + expect(textFileBag(n)).toEqual({ Complexity: 'complex', DimensionsMode: 'Variable', Max: 100, Min: 0 }); + }); + + it('does not write an enum back just because the user chose MATLAB\'s default', () => { + // Setting Complexity to 'real' on a file that never declared it leaves the file saying + // what it always said. This is the case the plain `!== default` gate gets right and a + // truthiness gate gets wrong. + const n = valueType({}); + expect(n.setProperty('complexity', 'real')).toBe(true); + expect(binaryPath(n)).toEqual({}); + }); + + it('does not gain an enum key when an edit is cleared again', () => { + // A clear stores '', which is not a value either property has, so it means absence — + // the same thing the file already said. Without the `this.X &&` half of the gate this + // writes `Complexity: ""`, an empty char where MATLAB expects an enumeral. + const n = valueType({}); + expect(n.setProperty('complexity', 'complex')).toBe(true); + expect(n.setProperty('complexity', '')).toBe(true); + expect(binaryPath(n)).toEqual({}); + expect(textFileBag(n)).toEqual({}); + }); + + it('writes a cleared bound as MATLAB\'s empty, not as the value the file held', () => { + // Emptying the Minimum box must not save the old number back — the bound would return + // on reopen, with the row blank until then. + const n = valueType({ Min: 5, Max: 9 }); + expect(n.setProperty('Min', '')).toBe(true); + expect(binaryPath(n).Min).toEqual([]); + expect(textFileBag(n).Min).toEqual([]); + expect(binaryPath(n).Max).toBe(9); + }); + + it('writes the unit under the spelling the file used', () => { + // A ValueType's own key is `Unit`, but a file that came in spelled `DocUnits` keeps + // that spelling: writing both would leave MATLAB two units to choose between. + expect(binaryPath(valueType({ Unit: 'm' }))).toEqual({ Unit: 'm' }); + expect(binaryPath(valueType({ DocUnits: 'm/s' }))).toEqual({ DocUnits: 'm/s' }); + expect(textFileBag(valueType({ DocUnits: 'm/s' }))).toEqual({ DocUnits: 'm/s' }); + }); +}); + +for (const format of ['json', 'binary'] as SlddFormat[]) { + describe(`ValueType value properties — .sldd round trip (${format})`, () => { + // The half that decides whether an edit reaches the FILE. Both writers serialize from + // the property bag, not from the node's fields, so an edit never copied across is + // stored, displayed and then lost on save — with our own reader agreeing with us + // afterwards, because it re-reads what the file still holds. + function freshValueType(tag: string) { + const uri = `test://vt-props-${format}-${tag}.sldd`; + const model = loadModel(format, 'params.sldd', uri); + return { model, entry: entryByName(model, uri, 'MyValueType') }; + } + + it('an edited Complexity and DimensionsMode survive serialize + re-parse', () => { + const { model, entry } = freshValueType('enums'); + expect(entry.setProperty('complexity', 'complex')).toBe(true); + expect(entry.setProperty('dimensionsMode', 'Variable')).toBe(true); + + const fresh = reparseEntry(serializeModel(model, format), format, 'params.sldd', 'MyValueType'); + expect(fresh.Complexity).toBe('complex'); + expect(fresh.DimensionsMode).toBe('Variable'); + }); + + it('an edited Min/Max survives serialize + re-parse, and a clear survives as no bound', () => { + const { model, entry } = freshValueType('bounds'); + expect(entry.setProperty('Min', '0')).toBe(true); + expect(entry.setProperty('Max', '100')).toBe(true); + + const edited = reparseEntry(serializeModel(model, format), format, 'params.sldd', 'MyValueType'); + expect(edited.Min).toBe(0); + expect(edited.Max).toBe(100); + + // Cleared on the node that belongs to `model`, since that is the model serialized + // below — `edited` is a node in the fresh model reparseEntry just built. + expect(entry.setProperty('Min', '')).toBe(true); + const cleared = reparseEntry(serializeModel(model, format), format, 'params.sldd', 'MyValueType'); + expect(cleared.Min).toBeUndefined(); + expect(cleared.Max).toBe(100); + }); + + it('a ValueType nobody edited keeps exactly the keys it arrived with', () => { + const { entry } = freshValueType('clean'); + const arrived = Object.keys(entry.serial._properties as Record); + expect(Object.keys(binaryPath(entry))).toEqual(arrived); + expect(Object.keys(textFileBag(entry))).toEqual(arrived.filter((k) => k !== '_id')); + }); + }); +} From a5590b640c78f1343a50f3363e5be2e99746f829 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Wed, 16 Sep 2026 16:39:25 -0400 Subject: [PATCH 2/6] [wip] Surface the flat properties MATLAB models on enums, lookup tables and config sets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A parity check against the MATLAB Data Explorer app asked what each class has that we do not model. Four answers, all measured off dictionaries MATLAB R2027a wrote rather than read off adapter code. Enums gain IsTunableInCode, which SLEnum.getCodegenPropertyNames appends under the OpaqueEnum feature. We cannot read a feature flag out of a file, so the row is always shown. Simulink.LookupTable and Simulink.Breakpoint had almost nothing: a lookup table carried only breakpointsSpecification and a breakpoint carried none at all. Both gain CoderInfo.StorageClass, the three StructTypeInfo keys, and SupportTunableSize; the lookup table also gains the different-breakpoint-sizes flag. StructTypeInfo is a nested MATLAB object, which resolveSourcePath already traverses the way signal.json reaches CoderInfo.CustomAttributes.DataScope. StorageClass is shared with Simulink.Parameter and Simulink.Signal, and the two allowed lists are not nested: a lookup table accepts seventeen values but NOT SimulinkGlobal or Custom, both of which the shared six offer. So the two classes override `options` per class rather than widening the shared descriptor — widening it would offer Custom on a lookup table, and narrowing it would break Parameter and Signal. trySetSchemaProperty validates select writes against that list, so it is an enforcement point and a test pins both directions. Config sets are narrowed to what a file actually carries. MATLAB's PI also shows StartTime, StopTime and SystemTargetFile, and all three are real parameters, but a dump of every era fixture found the modern path nests them in a heterogeneous Simulink.ConfigComponent array no fixed sourcePath can address, values at their default are not written at all, and one era carries none of them. Adding them would manufacture permanently blank rows — the defect this work removes. So Simulink.ConfigSet gains Description and Simulink.ConfigSetRef gains Description and SourceName. Both config-set nodes had to gain a Description FIELD, not just a layout entry: `description` is in schemaBridge's ATOM_BY_KEY, so the row reads the node field, and both constructors dropped their props argument. A layout entry alone would have reproduced the ValueType bug exactly. SourceName needs no field — one fixed sourcePath covers both eras because SlxParser already normalizes WSVarName into SourceName before a node is built. The property-parity test now looks a nested MATLAB property up QUALIFIED by its parent (StructTypeInfo.Name, not Name) before falling back to its leaf-name fold, so a sub-property cannot be satisfied by some other class's row of the same name. --- src/datamodel/node/data/ConfigSetNode.ts | 26 +- src/datamodel/node/data/ConfigSetRefNode.ts | 29 +- .../node/data/Simulink.BusElement.md | 30 +- src/datamodel/node/data/Simulink.ValueType.md | 118 +++++- src/datamodel/schema/classes/breakpoint.json | 12 +- src/datamodel/schema/classes/configSet.json | 2 +- .../schema/classes/configSetRef.json | 4 +- src/datamodel/schema/classes/enumType.json | 5 +- src/datamodel/schema/classes/lookupTable.json | 14 +- src/datamodel/schema/props/core.json | 3 +- test/configSetSchemaProps.test.ts | 352 ++++++++++++++++++ test/enumIsTunableInCode.test.ts | 267 +++++++++++++ test/lookupTableFlatProps.test.ts | 331 ++++++++++++++++ .../fidelity/hostnodes.fidelity.test.ts | 17 +- test/parity/matlab/schemaProps.test.ts | 66 +++- test/schema/piGeneralAllNodes.test.ts | 21 +- 16 files changed, 1240 insertions(+), 57 deletions(-) create mode 100644 test/configSetSchemaProps.test.ts create mode 100644 test/enumIsTunableInCode.test.ts create mode 100644 test/lookupTableFlatProps.test.ts diff --git a/src/datamodel/node/data/ConfigSetNode.ts b/src/datamodel/node/data/ConfigSetNode.ts index 2aa7630..286708a 100644 --- a/src/datamodel/node/data/ConfigSetNode.ts +++ b/src/datamodel/node/data/ConfigSetNode.ts @@ -4,8 +4,18 @@ import type { PropClass } from '../BaseNode.js'; import type BaseNode from '../BaseNode.js'; import PropName from '../../prop/PropName.js'; import PropDataType from '../../prop/PropDataType.js'; +import PropDescription from '../../prop/PropDescription.js'; const CLASS_NAME = 'Simulink.ConfigSet'; export default class ConfigSetNode extends SimulinkObjectNode { + // A real top-level property of a MATLAB config set, and the ONLY one of the four its + // Property Inspector shows that a file reliably carries — see configSetSchemaProps.test.ts + // for why StartTime/StopTime/SystemTargetFile/SourceLocation are deliberately unmodeled. + // The field is not optional decoration: `description` is a key in schemaBridge's + // ATOM_BY_KEY, so the layout row reads THIS field rather than the schema descriptor's + // sourcePath. Until it existed the row rendered blank and, because toPIObject adds every + // layout key to shownKeys, the raw Description was suppressed from the "Other" group as + // well — a Description in the file, unreachable in the UI. + Description: string; // The config set's own Name property. In a .sldd the entry name and this // property are the same string — both parse paths build the node with // _properties.Name equal to the entry name — so this is a view of `name` @@ -18,18 +28,22 @@ export default class ConfigSetNode extends SimulinkObjectNode { // parser (which knows the active state); undefined on the SLDD path, where // it is treated as inactive — the SLDD icon is unchanged. active?: boolean; - constructor(name: string, parent: BaseNode | null, props: Record, serial: Record) { super(name, parent, serial); } + constructor(name: string, parent: BaseNode | null, props: Record, serial: Record) { super(name, parent, serial); this.Description = (props.Description as string) || ''; } get icon(): string { return this.active ? 'check_settings' : 'settings'; } get className(): string { return CLASS_NAME; } // A ConfigSet has no scalar "value" — the Value column is empty and not editable. get displayValue(): string { return ''; } get valueEditable(): boolean { return false; } - getProperties(): PropClass[] { return [PropName, PropDataType]; } + getProperties(): PropClass[] { return [PropName, PropDataType, PropDescription]; } // PI layout: schema-driven "General" group (classes/configSet.json). - // UNGATED: a config set MATLAB can load has to be able to say what it is called, so the - // key is written whatever the file carried — and because it is a view of `name`, a - // renamed entry saves under the new name on both paths. - _serializedOverrides(): Record { return { Name: this.ConfigName }; } + // Name is UNGATED: a config set MATLAB can load has to be able to say what it is called, + // so the key is written whatever the file carried — and because it is a view of `name`, a + // renamed entry saves under the new name on both paths. Description is GATED, on the same + // terms as every other Description in this cluster: a config set whose file never carried + // one must not gain an empty one on save, or opening a dictionary and saving it with no + // edits produces a diff in source control. Order follows AliasTypeNode — the identity key + // the class owns, then the gated description. + _serializedOverrides(): Record { return Object.assign({ Name: this.ConfigName }, this._gatedProps({ Description: this.Description })); } static get defaultName(): string { return 'Configuration'; } static createDefault(name: string, parent: BaseNode | null): ConfigSetNode { const rawVal = ConfigSetNode._defaultRawVal(CLASS_NAME, { Name: name || 'Configuration' }); const props = ConfigSetNode._propsOf(rawVal); return new ConfigSetNode(name, parent, props, { _rawVal: rawVal, _properties: props } as Record); } static parse(rawVal: Record, name: string, parent: BaseNode | null): ConfigSetNode { const props = ConfigSetNode._propsOf(rawVal); return new ConfigSetNode(name, parent, props, { _rawVal: rawVal, _properties: props } as Record); } diff --git a/src/datamodel/node/data/ConfigSetRefNode.ts b/src/datamodel/node/data/ConfigSetRefNode.ts index 82d9db6..dc7d9d2 100644 --- a/src/datamodel/node/data/ConfigSetRefNode.ts +++ b/src/datamodel/node/data/ConfigSetRefNode.ts @@ -4,23 +4,40 @@ import type { PropClass } from '../BaseNode.js'; import type BaseNode from '../BaseNode.js'; import PropName from '../../prop/PropName.js'; import PropDataType from '../../prop/PropDataType.js'; +import PropDescription from '../../prop/PropDescription.js'; const CLASS_NAME = 'Simulink.ConfigSetRef'; export default class ConfigSetRefNode extends SimulinkObjectNode { + // The reference's whole content: the name of the config set it points AT. Stored and + // saved from the start; it now also has a PI row, through the schema descriptor + // `sourceName` (classes/configSetRef.json) rather than through a node atom — which is + // sound only because the row is read-only. The descriptor hydrates + // `serial._properties.SourceName`, so an EDIT would land on this field and leave the row + // showing the untouched bag; if this ever becomes editable it has to move to an atom, for + // the reason ValueTypeNode.getPILayout records at length. + // + // One fixed sourcePath is enough despite the era-varying spelling (`SourceName` in + // R2021a+, `WSVarName` in R2018a and earlier) because the normalization happens upstream: + // SlxParser reads either into ParsedConfigSet.sourceName and + // ModelSectionNode.addConfigSetEntry writes it back as `props.SourceName`, so by the time + // a node sees it the key is always spelled one way. SourceName: string; + // See ConfigSetNode.Description — the same field for the same reason (the `description` + // layout key resolves to an atom that reads the node field, not the schema sourcePath). + Description: string; // See ConfigSetNode.active — set by the SLX parser only. active?: boolean; - constructor(name: string, parent: BaseNode | null, props: Record, serial: Record) { super(name, parent, serial); this.SourceName = (props.SourceName as string) || ''; } + constructor(name: string, parent: BaseNode | null, props: Record, serial: Record) { super(name, parent, serial); this.SourceName = (props.SourceName as string) || ''; this.Description = (props.Description as string) || ''; } get icon(): string { return this.active ? 'check_configurationReference' : 'configurationReference'; } get className(): string { return CLASS_NAME; } // A ConfigSetRef has no scalar "value" — the Value column is empty and not editable. get displayValue(): string { return ''; } get valueEditable(): boolean { return false; } - getProperties(): PropClass[] { return [PropName, PropDataType]; } + getProperties(): PropClass[] { return [PropName, PropDataType, PropDescription]; } // PI layout: schema-driven "General" group (classes/configSetRef.json). - // UNGATED, on the same terms as a ConfigSet's Name: a reference that cannot say what it - // points at is not a reference, so the key is written even as the empty string a - // half-built entry carries. - _serializedOverrides(): Record { return { SourceName: this.SourceName }; } + // SourceName is UNGATED, on the same terms as a ConfigSet's Name: a reference that cannot + // say what it points at is not a reference, so the key is written even as the empty string + // a half-built entry carries. Description is GATED — see ConfigSetNode._serializedOverrides. + _serializedOverrides(): Record { return Object.assign({ SourceName: this.SourceName }, this._gatedProps({ Description: this.Description })); } static get defaultName(): string { return 'ConfigSetRef'; } static createDefault(name: string, parent: BaseNode | null): ConfigSetRefNode { const rawVal = ConfigSetRefNode._defaultRawVal(CLASS_NAME, { SourceName: '' }); const props = ConfigSetRefNode._propsOf(rawVal); return new ConfigSetRefNode(name, parent, props, { _rawVal: rawVal, _properties: props } as Record); } static parse(rawVal: Record, name: string, parent: BaseNode | null): ConfigSetRefNode { const props = ConfigSetRefNode._propsOf(rawVal); return new ConfigSetRefNode(name, parent, props, { _rawVal: rawVal, _properties: props } as Record); } diff --git a/src/datamodel/node/data/Simulink.BusElement.md b/src/datamodel/node/data/Simulink.BusElement.md index 9cb9539..496ab0d 100644 --- a/src/datamodel/node/data/Simulink.BusElement.md +++ b/src/datamodel/node/data/Simulink.BusElement.md @@ -83,11 +83,28 @@ Both are surfaced as `editor: 'select'` dropdowns over MATLAB's own enum - `elem.DimensionsMode = 'fixed'` / anything outside the enum → **"There is no enumerated value named 'fixed'."** - `elem. = ''` → treated as a CLEAR, not a rejection. Not MATLAB's behavior - (MATLAB refuses `''` too) but required by ours: an element that never carried the - property reads as `''`, so that is the prior value undo submits, and refusing it - would leave the undo of a legitimate edit silently unapplied. A cleared property is - omitted from the serialized bag entirely, so it cannot reach a file as an empty - enumeral. + (MATLAB refuses `''` too) but required by ours: `''` is what a user emptying the cell + submits, and `DataModel.editProperty` captures the prior value for UNDO, so refusing + it would leave the undo of a clear silently unapplied. A cleared property is omitted + from the serialized bag entirely — the write-back gate reads `''` as absence — so it + cannot reach a file as an empty enumeral. + +### Complexity / DimensionsMode — displayed default vs saved key + +An element whose file declares neither property DISPLAYS MATLAB's own default for it, +`real` and `Fixed` (probed on a live `Simulink.BusElement` — the same pair +`Simulink.ValueType` defaults to, and NOT the `auto` a `Simulink.Signal` uses). MATLAB +shows a value for both on every element it has, because the object always has one; only +the file is silent. Before this the fallback was `''`, so the two rows read blank. + +These are DISPLAY values only. `_applyElementOverrides` writes each key only when the +FILE carried it or the live value differs from that same default, so an element the file +left silent still saves silent: open a dictionary, change nothing, save, and no element +gains a key. The gate cannot be a truthiness test (`'Complexity' in sp || this.Complexity`, +which is what it was) — with a non-empty default that is always true, and every untyped +element in every dictionary would silently gain both keys. The extra `this.X &&` covers +the CLEAR above, `''` not being a value either enum has. +Test: `test/busElementDisplayDefaults.test.ts`. The casing is not normalized in either direction — MATLAB wrote 'real' lower case and 'Fixed' capitalized, and it refuses the other spelling of each, so a @@ -126,7 +143,8 @@ value. Test: round-trip in `test/parity/fidelity/element.fidelity.test.ts`. - `BusElementNode.setProperty('complexity'|'dimensionsMode', ...)` routes through - `_rejectUnknownEnumeral` (override in BusNode.ts), which reads the legal set off the + `_rejectUnknownEnumeral` (on `DataNode`, beside `_setMinMax`; it started here with one + caller and moved when `Simulink.ValueType` needed the same rule), which reads the legal set off the prop atom's `readOptions` — the same call the dropdown is built from, so the offered choices and the accepted values cannot drift — and returns `{error, reason: "There is no enumerated value named 'X'."}`. diff --git a/src/datamodel/node/data/Simulink.ValueType.md b/src/datamodel/node/data/Simulink.ValueType.md index 7265359..1901c0b 100644 --- a/src/datamodel/node/data/Simulink.ValueType.md +++ b/src/datamodel/node/data/Simulink.ValueType.md @@ -4,34 +4,82 @@ **Node class:** `ValueTypeNode` (`src/datamodel/node/data/ValueTypeNode.ts`) **MATLAB class:** `Simulink.ValueType` -**Editable in our UI:** no (read-only pass-through; only Name and Description editable) +**Editable in our UI:** yes (Name, Description, Min, Max, Complexity, DimensionsMode) **Verified against:** MATLAB R2027a (probe_class('Simulink.ValueType')) +**Partly unverified:** the Min / Max / Complexity / DimensionsMode unlock is verified +in-process only. There is no MATLAB re-open gate for those four — see "Open questions". ## Overview A Simulink.ValueType defines a reusable value-type specification (data type, unit, dimensions, complexity, min/max bounds) that can be applied to signals, states, and -parameters. In our UI the ValueType surfaces as a read-only entry: the Value column -is empty and not editable (`valueEditable = false`). The Data Type column shows the -underlying DataType property (defaulting to 'double'). The Property Inspector shows -Name, Value (empty), DataType, and Description. Only Name and Description are -editable via the UI. +parameters. In our UI the ValueType has no scalar "value": the Value column is empty and +not editable (`valueEditable = false`). The Data Type column shows the underlying +DataType property (defaulting to 'double'). The Property Inspector shows Name, DataType, +Dimensions, Complexity, DimensionsMode, Min, Max, Unit, and Description — the same +value-property surface a `Simulink.BusElement` gets, and for the same reason: MATLAB +models these properties on the object, so a UI that hides them is hiding data the file +carries. ## Property table -| Property | MATLAB type | SetAccess | Editable here | Serialized key (JSON / binary) | Editor | Allowed values / constraint | -|-------------|-------------|-----------|---------------|--------------------------------|--------|-----------------------------| -| Name | char | (entry) | yes | name / name | text | Valid MATLAB identifier, unique in namespace | -| Description | char | public | yes | Description / Description | text | Any string | -| DataType | char | public | no (label) | DataType / DataType | label | Any string (free-form; MATLAB validates downstream) | -| Unit | char | public | no | Unit / Unit | — | Any string (SI or custom unit) | -| Min | double / [] | public | no | Min / Min | — | Finite real double scalar, or [] | -| Max | double / [] | public | no | Max / Max | — | Finite real double scalar, or [] | -| Complexity | char (enum) | public | no | Complexity / Complexity | — | 'real' or 'complex' | -| Dimensions | double | public | no | Dimensions / Dimensions | — | Positive integer row vector | +| Property | MATLAB type | SetAccess | Editable here | Serialized key (JSON / binary) | Editor | Allowed values / constraint | +|----------------|-------------|-----------|---------------|--------------------------------|----------|-----------------------------| +| Name | char | (entry) | yes | name / name | text | Valid MATLAB identifier, unique in namespace | +| Description | char | public | yes | Description / Description | textArea | Any string | +| DataType | char | public | no (label) | DataType / DataType | label | Any string (free-form; MATLAB validates downstream) | +| Unit | char | public | no (label) | Unit / Unit | label | Any string (SI or custom unit) | +| Min | double / [] | public | yes | Min / Min | text | Finite real double scalar; `[]` clears | +| Max | double / [] | public | yes | Max / Max | text | Finite real double scalar; `[]` clears | +| Complexity | char (enum) | public | yes | Complexity / Complexity | select | 'real' or 'complex' (exact case); `''` clears | +| DimensionsMode | char (enum) | public | yes | DimensionsMode / DimensionsMode | select | 'Fixed' or 'Variable' (exact case); `''` clears | +| Dimensions | double | public | no (label) | Dimensions / Dimensions | label | Positive integer row vector | ## Non-obvious behavior (the reason this doc exists) +### The value properties were modelled but unreachable + +`schema/classes/valueType.json` already carried a layout naming `min`, `max`, `unit`, +`dimensions` and `complexity`, and every one of those rows rendered BLANK. Those keys +resolve through `schemaBridge`'s `ATOM_BY_KEY` to atoms that read node FIELDS, and +`ValueTypeNode` had no such fields — so each row read empty AND was added to `shownKeys`, +which had the "Other" catch-all suppress the raw source key as well. A `"Unit": "m"` in +the dictionary was therefore invisible in both panes at once +(`test/parity/artifacts/text/params.sldd`'s `MyValueType` is exactly that file). The fix +is the fields; the layout did not need to change. +Test: `test/valueTypeValueProps.test.ts`. + +### Unit is spelled `Unit`, not `DocUnits` + +MATLAB serializes a ValueType's unit as a flat `Unit` key, where a `Simulink.Parameter`'s +or `Simulink.Signal`'s is `DocUnits` (both measured off dictionaries MATLAB wrote). The +node reads both spellings so a file written either way displays, `Unit` first, and writes +back under whichever spelling the file used. `PropUnit` already declares +`sourceKeys = ['DocUnits', 'Unit']`, so neither spelling leaks into "Other". +The per-class `sourcePath: "Unit"` override in `valueType.json` says the same thing on the +schema side; it is pinned by a test that reads the resolved descriptor, because nothing in +the rendered UI depends on it today. + +### Complexity / DimensionsMode — displayed default vs saved key + +A ValueType that declares neither DISPLAYS MATLAB's default, `real` and `Fixed` — the same +pair `Simulink.BusElement` defaults to, and NOT the `auto` a `Simulink.Signal` uses. Two +places have to agree on that: `valueType.json` overrides the shared `dimensionsMode` +descriptor's `default` per class (which feeds the TABLE column via `schemaColumns`) and the +constructor's fallback feeds the Property Inspector. The test asserts the two AGREE rather +than asserting each alone. + +Display values only: `_serializedOverrides` writes each key only when the FILE carried it +or the live value differs from the default, so a ValueType the file left silent saves +silent. See the BusElement doc for why the gate cannot be a truthiness test. + +### No `*_internal` aliases + +Unlike `Simulink.BusElement`, none of these properties is read through a `*_internal` +alias: a probe wrote a ValueType with every property non-default and the dictionary came +back with flat keys only. That aliasing is specific to bus elements in SLX XML, so looking +for it here would be inventing a spelling MATLAB does not use. + ### Value column - A ValueType has no scalar "value" — `displayValue` returns `''` and @@ -52,7 +100,12 @@ editable via the UI. ## Allowed values (enums / comboboxes) -None exposed in our UI. Complexity ('real'/'complex') is read-only. +- Complexity → `['real', 'complex']` (`PropComplexity.readOptions`) +- DimensionsMode → `['Fixed', 'Variable']` (`PropDimensionsMode.readOptions`) + +Exact case, not normalized in either direction: MATLAB wrote 'real' lower case and 'Fixed' +capitalized and refuses the other spelling of each, so a case-insensitive match here would +produce a file MATLAB will not load. ## Validation mirrored in code @@ -60,6 +113,18 @@ None exposed in our UI. Complexity ('real'/'complex') is read-only. identifier, unique in namespace, max 63 chars, not a keyword). Test: `test/parity/fidelity/typedef.fidelity.test.ts`. +- Min / Max: `setProperty` routes to `DataNode._setMinMax`, the shared finite-real-double- + scalar rule (`{error, reason: "