-
Notifications
You must be signed in to change notification settings - Fork 46
Add function to disable console window #333
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
Add function to disable console window #333
Conversation
|
I will take a look asap.
For the time being I would say OwnConsoleWindow shall be restored to 0 on
MD_Close().
Otherwise it will probably crash when creating and closing a batch of
instances.
…On Wed, 22 Oct 2025, 23:33 Ryan Davies, ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In source/MoorDyn.cpp
<#333 (comment)>
:
> @@ -85,63 +88,65 @@ MoorDyn md_singleton = NULL;
int DECLDIR
MoorDynInit(const double x[], const double xd[], const char* infilename)
{
+ if (!disableConsoleWindow) {
@jtgrasb <https://github.com/jtgrasb> this needs to be changed to
disableConsole
disableConsoleWindow isn't declared and is why the tests are failing
—
Reply to this email directly, view it on GitHub
<#333 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMXKKDCBFBRBYTT7VMHXIL3Y7Z3TAVCNFSM6AAAAACJ5FXCMOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGNRXGYYTINRVGA>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
@jtgrasb, could you implement @sanguinariojoe suggestion for the close function? |
|
@sanguinariojoe To clarify, do you mean adding |
|
Yes, that is correct
…On Mon, 27 Oct 2025, 21:49 jtgrasb, ***@***.***> wrote:
*jtgrasb* left a comment (FloatingArrayDesign/MoorDyn#333)
<#333 (comment)>
@sanguinariojoe <https://github.com/sanguinariojoe> To clarify, do you
mean adding OwnConsoleWindow = 0 after this if statement?
if (OwnConsoleWindow == 1) {
std::cout << "press enter to close: " << std::endl;
std::cin.get();
FreeConsole();
}
—
Reply to this email directly, view it on GitHub
<#333 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMXKKCOVDMUMND2XMTDDBL3Z2AODAVCNFSM6AAAAACJ5FXCMOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTINJTGI2TANBYG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@jtgrasb, also it looks like some of the tests are failing because compilers are not liking that |
|
The pipeline is failing... But it has been failing since 28th August. I cannot see any commit that might triggered the issue. We should investigate that @RyanDavies19 Anyway, @jtgrasb increase the version here to 2.5.0 please. After that you can proceed to squash and merge guys (remember to put a sensible commit message according to the conventional commits standard) |
|
@sanguinariojoe @RyanDavies19 Thanks for the help. I think this PR should be good to merge. |
|
@jtgrasb I haven't had a chance to look over why the tests are still failing, but I'm fairly confident it isn't because of your changes. Do they pass when you run them locally? |
|
OK, Apparently the CMake fails are caused by GitHUB actions shifting macos-latest to arm64 arch (see here). Why is that a problem for math? no idea... Anyway, Both the Python wrapper and the Matlab wrapper shall be running just fine: https://github.com/core-marine-dev/MoorDyn/actions/runs/19419494577 So there is something introduced here which is breaking the wrappers. DO NOT MERGE UNTIL FIXED PLEASE. I will first try to fix the problems on the CI/CD and then I will focus on this (if not fixed yet) |
|
I think the problem is because the version increase, something is wrong with the versions reading... I will tackle that |
|
@jtgrasb please rebase dev, that should fix the issue |
abbdbf0 to
42b620d
Compare
|
@sanguinariojoe Thanks for the suggestions. I've rebased, hopefully that fixes the issues. |
|
Thanks @sanguinariojoe for the fix! All the tests are now passing so merging |
|
@RyanDavies19 , you should squash and merge, not merge directly. And you should set a conventional commit. I am adding an useless commit to document the changes |
|
Great, thanks for merging! I assume the plan is to release as v2.5.0 now? |
I was not planning to. But if it is OK with @RyanDavies19 ... It will be a storm of shit with all the changes on github actions and so on, you will see... |
|
@sanguinariojoe thanks for the reminder on conventional commits. @jtgrasb do you need a release for WECSim? |
|
@RyanDavies19 On our end, it would be nice to have a new release to avoid the crashing of MATLAB which disrupts our CI. As suggested by @sanguinariojoe, I've already increased the version number to v2.5.0 in this PR. |
This PR adds a function that can be called to disable the console window when using the v1 API. This will allow WEC-Sim to disable the console window before initializing MoorDyn, hopefully resolving issues with MATLAB crashing. See #327.