Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve routing logic #425

Merged
merged 13 commits into from
Jan 23, 2025
Merged

Conversation

AntoineGautier
Copy link
Collaborator

@AntoineGautier AntoineGautier commented Jan 13, 2025

TODO

Description

This PR

  • changes the annotations used to mark template classes and their packages, see
    The template discovery process relies on a grep for the following hierarchical class annotations:
    - `__ctrlFlow(routing="root()`: Marks the lop-level package containing all templates, e.g.,
    `Buildings.Templates`
    - `__ctrlFlow(routing="template")`: Marks individual template classes, e.g.,
    `Buildings.Templates.AirHandlersFans.VAVMultiZone`
    All template classes, and all packages between the root package and the template classes
    are considered as "entry points" and added to the `templateNodes` array of [loader.ts](./loader.ts).
    This means that the class URI ultimately dictates the explorer tree structure,
    starting from the "root" package which must be unique inside a library.
    and joint PR in MBL Change __ctrlFlow annotations modelica-buildings#4093
  • stores all the data needed to support a multiple tier tree structure within the templateNodes array, see
    // Master list of TemplateNode objects representing all templates and packages.
    // It is intended to be used in the future when the UI can handle nested packages.
    const templateNodes: TemplateNode[] = [];
  • prunes the entry points to keep only the immediate parent package of each template class, due to the current limitation of the UI that does not support a multiple tier tree structure, as explained in the linked issue), see
    // Fix for https://github.com/lbl-srg/ctrl-flow-dev/issues/422
    // Currently the UI does not support nested system types.
    // Therefore, we only add the Modelica class name of the containing package.
    const type = parser.findElement(path.join("."));
    if (type && type.entryPoint) {
    if (!type.description) {
    console.error(
    "Error: missing class description string in entry point " +
    type.modelicaPath,
    );
    process.exit(1);
    }
    const systemType = {
    description: type.description,
    modelicaPath: type.modelicaPath,
    };
    this.systemTypes.unshift(systemType);
    systemTypeStore.set(type.modelicaPath, systemType);

Related Issue(s)

#422

Dependency to #424

Testing

With these changes, and using a temporary branch of MBL that adds __ctrlFlow(routing="template") to both the HP plant template and its controller, we get the following template list in the UI (to be compared to the screenshot from the issue page).
image

@AntoineGautier AntoineGautier changed the base branch from main to staging January 13, 2025 08:49

The executed command looks as follows:
- `__ctrlFlow(routing="root()`: Marks the lop-level package containing all templates, e.g.,
Copy link
Collaborator

Choose a reason for hiding this comment

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

lop-level --> top-level

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes

});

it("Extracts two templates and three Template types to be in stores", () => {
const templates = [...getTemplates()];
expect(templates.length).toBe(2);

const systemTypes = [...getSystemTypes()];
expect(systemTypes.length).toBe(3);
console.log(systemTypes);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to keep this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No. I've removed it.

Copy link
Collaborator

@JayHuLBL JayHuLBL left a comment

Choose a reason for hiding this comment

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

It looks good. Please see the inline comments.

@AntoineGautier
Copy link
Collaborator Author

@JayHuLBL Thanks!

@AntoineGautier AntoineGautier marked this pull request as draft January 22, 2025 13:39
@AntoineGautier AntoineGautier marked this pull request as ready for review January 23, 2025 14:14
@JayHuLBL
Copy link
Collaborator

@AntoineGautier Only one small question. Other than that, it looks good to me.

@JayHuLBL JayHuLBL merged commit 628dc0f into lbl-srg:staging Jan 23, 2025
1 check passed
JayHuLBL added a commit that referenced this pull request Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants