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

Enable launching MATLAB desktop from the extern launcher #6366

Merged
merged 14 commits into from
Sep 25, 2023

Conversation

nilsjor
Copy link
Contributor

@nilsjor nilsjor commented Aug 28, 2023

Description
This will add features as described in #6362 (comment)

Related Issues
This pull-request fixes issue #6362

Tasks
Add the list of tasks of this PR.

  • Add the --interactive option to the webots-controller which will launch MATLAB in interactive (debug) mode.
  • Remove references to deprecated commands in example files.
  • Change the documentation to reflect the new workflow.

Documentation
The following pages have been changed:

@nilsjor nilsjor requested a review from a team as a code owner August 28, 2023 16:30
@nilsjor nilsjor changed the title Enable launching MATLAB desktop from exter launcher Enable launching MATLAB desktop from the extern launcher Aug 28, 2023
@omichel
Copy link
Member

omichel commented Aug 29, 2023

It looks very good to me. Remain to address the documentation.

@omichel omichel marked this pull request as draft August 29, 2023 06:59
@omichel omichel added the enhancement Implementation of a minor feature label Aug 29, 2023
@omichel omichel added this to the R2023b-rev1 milestone Aug 29, 2023
nilsjor

This comment was marked as duplicate.

@nilsjor nilsjor force-pushed the matlab-desktop-extern branch from 9a44085 to f601d2c Compare August 29, 2023 11:56
@nilsjor nilsjor marked this pull request as ready for review August 29, 2023 12:33
@nilsjor
Copy link
Contributor Author

nilsjor commented Aug 29, 2023

@matl-hsk could you please help testing on macOS?

  1. Open the "mir100" world in webots.
  2. Change the "controller" field of Mir100 (the white AGV) to <extern>.
  3. Run the Webots simulation. Nothing should happen until an extern controller has connected.
  4. In a new Terminal window, call the following:
webots-controller --interactive ~/webots/projects/robots/mir/mir100/controllers/keyboard_matlab/keyboard_matlab.m
  1. Wait until MATLAB opens. Click "Continue" in the debugging toolstrip.
  2. The simulation should start. Click on the Mir100 robot. Try to drive it with the arrow keys of your keyboard.

@matl-hsk
Copy link
Contributor

matl-hsk commented Sep 6, 2023

sorry for the delay.

I've tested your branch but it does not open Matlab, neither with the interactive option nor without. I can however

  • start Matlab and run the launcher manually (similar to how you described it in the issue)
  • start Matlab in batch mode from webots

@nilsjor
Copy link
Contributor Author

nilsjor commented Sep 7, 2023

sorry for the delay.

I've tested your branch but it does not open Matlab, neither with the interactive option nor without. I can however

* start Matlab and run the launcher manually (similar to how you described it in the issue)

* start Matlab in batch mode from webots

Can you provide the output from the terminal?

Have you tried using webots-controller for any other controller, python, etc.?

@matl-hsk
Copy link
Contributor

matl-hsk commented Sep 8, 2023

unfortunately it does not give any hint on the error. I looked at the code but could not find an issue quickly.

The started controller targets a local instance (ipc protocol) of Webots with port number 1234.
Targeting the only robot waiting for an extern controller.
Running MATLAB in interactive mode...

@nilsjor
Copy link
Contributor Author

nilsjor commented Sep 8, 2023 via email

@matl-hsk
Copy link
Contributor

matl-hsk commented Sep 8, 2023

Have you tried providing the matlab path, perhaps it cannot find it?

This was it! From webots itself it can find Matlab and from the terminal I can start it as well.

Suggestion: Instead of "Running Matlab ..." it should rather print an error when it cannot find Matlab.

Two more issues

  • The "-wait" option for Matlab is only available for windows but not on Linux or Mac.
  • For the webots command a small script to start it is created in the webots home folder but not for the webots-controller. For the latter one has to dig into the folder "Contents/MacOS" to find it. I would rather make this consistent. Or is this on purpose?

@nilsjor
Copy link
Contributor Author

nilsjor commented Sep 8, 2023

Suggestion: Instead of "Running Matlab ..." it should rather print an error when it cannot find Matlab.

This is a great observation! But the solution lies outside the scope for this PR. As you saw, this bug appears also for non-interactive use... If you you could, please open another Issue; "No error generated when webots-controller is unable to find executable" or similar ;)

For the future, I believe the underlying reason has to do with the configuration of your MSYS. You could check the PATH variables, it is likely that you are missing something.

The "-wait" option for Matlab is only available for windows but not on Linux or Mac.

That's interesting. Perhaps it is already the default behaviour on unix that the MATLAB window is blocking... Could you test that for me? Just run matlab in a terminal and check if control is returned at any point.

For the webots command a small script to start it is created in the webots home folder but not for the webots-controller. For the latter one has to dig into the folder "Contents/MacOS" to find it. I would rather make this consistent. Or is this on purpose?

This is also a separate Issue, feel free to report it if you want. It could also have to do with your MSYS installation, it would have to be reproduced by someone else.

Finally—regarding the features of this PR—did you get the interactive debugging to work? Were you able to reproduce these 6 steps?

  1. Open the "mir100" world in webots.
  2. Change the "controller" field of Mir100 (the white AGV) to <extern>.
  3. Run the Webots simulation. Nothing should happen until an extern controller has connected.
  4. In a new Terminal window, call the following:
webots-controller --interactive ~/webots/projects/robots/mir/mir100/controllers/keyboard_matlab/keyboard_matlab.m
  1. Wait until MATLAB opens. Click "Continue" in the debugging toolstrip.
  2. The simulation should start. Click on the Mir100 robot. Try to drive it with the arrow keys of your keyboard.

Print details only if valid matlab installation is found
@matl-hsk
Copy link
Contributor

matl-hsk commented Sep 8, 2023

On topic:

  • Yes, Matlab blocks the terminal until it is closed.
  • It works all as intended when I specify the Matlab path.

Off topic:

  • I understand that you want to prevent feature creep in this PR - okay with me to not fix the other issues.
  • Regarding the path: Matlab is in the user path (usr/local/bin) but apparently the app is started as another user or does not use the path variable. I am mostly a Linux user and don't know how this is handled on Mac.

@nilsjor
Copy link
Contributor Author

nilsjor commented Sep 8, 2023

  • Yes, Matlab blocks the terminal until it is closed.

  • It works all as intended when I specify the Matlab path.

Great, thanks!

As a last touch, I moved the printout so it will only appear if a valid installation is found. I also removed -wait from non-windows versions.

Out of curiosity, I had a look at why it does not warn, and I found this

#ifndef __APPLE__
if (directory == NULL) {
fprintf(stderr, "No installation of MATLAB available.\n");
return false;
}
#endif

It seems that the error is suppressed only on macOS. I could not tell you why... Please include this in your Issue, if you open one.

@nilsjor
Copy link
Contributor Author

nilsjor commented Sep 19, 2023

Hello again. What else is needed to go ahead with this? I see it is still pending reviewers.
@ygoumaz pehaps you can have a look, since I know you are familiar with the webots-controller launcher?

I have left some explanations on the important files:

  • src/controller/launcher/webots_controller.c
  • lib/controller/matlab/launcher.m

Other than that, it's just the documentation and removed references to desktop in some examples (thanks matl-hsk!)

@omichel
Copy link
Member

omichel commented Sep 19, 2023

I did a first review a while ago, but my requests for change were not yet taken into account or answered.

src/controller/launcher/webots_controller.c Outdated Show resolved Hide resolved
src/controller/launcher/webots_controller.c Outdated Show resolved Hide resolved
src/controller/launcher/webots_controller.c Outdated Show resolved Hide resolved
src/controller/launcher/webots_controller.c Outdated Show resolved Hide resolved
@matl-hsk
Copy link
Contributor

Sorry for the late reply - I was too busy with other stuff. Thanks for the reminder.

Code looks good to me but that does not work as intended:

As a last touch, I moved the printout so it will only appear if a valid installation is found. I also removed -wait from non-windows versions.

I just tried without specifying the Matlab path and it still prints "starting matlab" although it cannot find it. Until a proper solution is available, I'd suggest to change the phrase to give a hint that if nothing happens one should try to specify the path to matlab. (Hope that fits into the PR.)

@nilsjor
Copy link
Contributor Author

nilsjor commented Sep 19, 2023

I just tried without specifying the Matlab path and it still prints "starting matlab" although it cannot find it. Until a proper solution is available, I'd suggest to change the phrase to give a hint that if nothing happens one should try to specify the path to matlab. (Hope that fits into the PR.)

Thanks again for the feedback! I am still of the opinion that this is outside the scope of this PR. It seems to me that the problem lies somewhere in the function get_matlab_path(), which is unchanged in this PR.

To summarize, could you confirm the following:

  1. Running webots-controller without interactive and without matlab-path: MATLAB never opens.
  2. Running webots-controller with interactive and without matlab-path:
    "Running MATLAB in interactive mode..." ...but MATLAB never opens.
  3. Running webots-controller without interactive and with correct matlab-path: MATLAB runs in batch mode.
  4. Running webots-controller with interactive and with correct matlab-path:
    "Running MATLAB in interactive mode..." and then MATLAB opens in interactive mode.

It appears that the launcher skips the following lines, even though the function should return false if there is not installation in the default path:

// If no MATLAB installation path was given in command line, check in default installation folder
if (!matlab_path && !get_matlab_path())
return -1;

The expected behavior here is that the launcher exits and you get:

fprintf(stderr, "No installation of MATLAB available.\n");
return false;

Could you please create a new issue, call it "macOS: No error generated when webots-controller cannot find executable".

There, please list the steps needed to reproduce it on the webots R2023b release (i.e., without this PR), including the path to your installed version of MATLAB, macOS, etc. Also describe if the launcher exits or if you need to Ctrl+C to return control.

Thanks!

@matl-hsk
Copy link
Contributor

  1. Running webots-controller without interactive and without matlab-path: MATLAB never opens.
  2. Running webots-controller with interactive and without matlab-path:
    "Running MATLAB in interactive mode..." ...but MATLAB never opens.
  3. Running webots-controller without interactive and with correct matlab-path: MATLAB runs in batch mode.
  4. Running webots-controller with interactive and with correct matlab-path:
    "Running MATLAB in interactive mode..." and then MATLAB opens in interactive mode.
    all correct

@matl-hsk
Copy link
Contributor

Could you please create a new issue, call it "macOS: No error generated when webots-controller cannot find executable".

Done and assigned you, @nilsjor.

omichel
omichel previously approved these changes Sep 22, 2023
Copy link
Member

@omichel omichel left a comment

Choose a reason for hiding this comment

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

Thank you both.

@nilsjor nilsjor merged commit 0e1ab73 into cyberbotics:master Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Implementation of a minor feature
Development

Successfully merging this pull request may close these issues.

3 participants