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

Update WPFCLI to add keep alive startup param handling #12156

Merged
merged 14 commits into from
Apr 11, 2022

Conversation

saintentropy
Copy link
Contributor

@saintentropy saintentropy commented Oct 18, 2021

Purpose

The purpose of this pull request is to add the keep alive option to the WPFCLI. Originally this PR added the option to WPFCLI with an identical implementation to the CLI. That PR failed to work as the Thread.Sleep calls on the main thread interfered with the Dispatcher pattern utilized in the WPF Dynamo components. This implementation differs in that it starts the Dynamo Model and ViewModel on their own thread and the Main thread of the Console handles blocking with a more standard Console.ReadLine().

This PR also adds the ability for the WPFCLI running in keep alive mode to load view extensions as well as utilizing the Helix Watch3D implementation vs Default.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Reviewers

TBD

FYIs

@BogdanZavu

{
model = Dynamo.Applications.StartupUtils.MakeModel(true, cmdLineArgs.ASMPath);
var thread = new Thread(() => RunKeepAlive(cmdLineArgs));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a name for this thread so that is easily visible when debugging like the Scheduler thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Name added

{
StartupDaynamo(cmdLineArgs);

Console.WriteLine("-----------------------------------------");
Copy link
Contributor

Choose a reason for hiding this comment

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

Now I realize that we probably want to show the console only in debug mode ? But we can deal with this later in other PR.

@saintentropy saintentropy changed the title WIP Update WPFCLI to add keep alive start up param (version 2) Update WPFCLI to add keep alive startup param handling Apr 4, 2022
}
else
{
model = Dynamo.Applications.StartupUtils.MakeModel(true);
model = Dynamo.Applications.StartupUtils.MakeModel(true, string.Empty, cmdLineArgs.AnalyticsInfo);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@QilongTang Hi Aaron. Is this the correct way to update the MakeModel usage from the obsoleted methods

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

@@ -13,6 +13,7 @@
<Reference Include="Microsoft.Practices.Prism">
<HintPath>..\..\extern\prism\Microsoft.Practices.Prism.dll</HintPath>
</Reference>
<Reference Include="WindowsBase" />
Copy link
Contributor

@QilongTang QilongTang Apr 4, 2022

Choose a reason for hiding this comment

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

Any reason why we need to add this? Would this prevent running WPF CLI on linux based system?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was necessary with the threading model required to load the ViewModel which makes sence since that also will not load on linux

@saintentropy saintentropy merged commit ced6306 into DynamoDS:master Apr 11, 2022
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.

3 participants