Mostly MacOS-related fixes (cyberbotics#6635)

* Support running qt_mac_installer.sh (and thus make) as regular user.

* Use NULL instead of kIOMainPortDefault or kIOMasterPortDefault to support compilation across SDK versions.

* Don't reuse socket that fails to connect because doing so causes "Invalid argument" error on MacOS.

* Fix long robot names causing connection problems in some environments.

* Improve test failure message when check_urdf is not installed.

* Ensure that the same format is used for the contexts associated with secondary windows, so that sharing will be successful.

* Use venv for aqt on MacOS.

* Fix ipc server name encoding to work with extern controllers.

* Fix crash on macos when exiting after running check_saved_hidden_parameters.wbt.

docs/reference/changelog-r2024.md

@@ -25,3 +25,6 @@ Released on December **th, 2023.
     - Made `supervisor_export_image` work even if using the `--minimize --no-rendering` options ([#6622](https://github.com/cyberbotics/webots/pull/6622)).
     - Fixed CA certificates needed by `test_suite.py` missing in Windows development environment ([#6628](https://github.com/cyberbotics/webots/pull/6628)).
     - Fixed to use a version of Qt that is still supported ([#6623](https://github.com/cyberbotics/webots/pull/6623)).
+    - Fixed connection errors when using long robot names in some environments ([#6635](https://github.com/cyberbotics/webots/pull/6635)).
+    - Fixed crash on macos when closing Webots under some circumstances ([#6635](https://github.com/cyberbotics/webots/pull/6635)).
+    - Fixed error on macos when when putting displays and cameras in separate windows ([#6635](https://github.com/cyberbotics/webots/pull/6635)).

projects/robots/gctronic/e-puck/plugins/remote_controls/e-puck_bluetooth/Serial.cpp

@@ -384,7 +384,8 @@ void Serial::updatePorts() {
     printf("IOServiceMatching failed.\n");
 
   CFDictionarySetValue(classesToMatch, CFSTR(kIOSerialBSDTypeKey), CFSTR(kIOSerialBSDRS232Type));
-  kernResult = IOServiceGetMatchingServices(kIOMainPortDefault, classesToMatch, &serialPortIterator);
+  // NULL is equivalent to kIOMainPortDefault or kIOMasterPortDefault but works across SDK versions
+  kernResult = IOServiceGetMatchingServices((mach_port_t)NULL, classesToMatch, &serialPortIterator);
   if (kernResult != KERN_SUCCESS)
     printf("IOServiceGetMatchingServices failed: %d\n", kernResult);
   io_object_t service;

scripts/install/qt_mac_installer.sh

@@ -9,8 +9,12 @@ if [[ -z "${WEBOTS_HOME}" ]]; then
 fi
 
 QT_VERSION=6.5.3
-pip install --no-input aqtinstall
+python3 -m venv ~/python_env_for_aqt
+source ~/python_env_for_aqt/bin/activate
+python3 -m pip install --upgrade pip
+python3 -m pip install --no-input aqtinstall
 aqt install-qt --outputdir $HOME/Qt mac desktop ${QT_VERSION} clang_64 -m qtwebsockets
+rm -rf ~/python_env_for_aqt
 
 # prepare Webots
 cd $WEBOTS_HOME

src/controller/c/robot.c

@@ -1081,21 +1081,33 @@ static void wb_robot_cleanup_devices() {
   }
 }
 
-static char *encode_robot_name(const char *robot_name) {
+static char *encode_robot_name(const char *robot_name, int chars_used) {
   if (!robot_name)
     return NULL;
 
   char *encoded_name = percent_encode(robot_name);
   int length = strlen(encoded_name);
   // the robot name is used to connect to the libController and in this process there are indirect
-  // limitations such as QLocalServer only accepting strings up to 106 characters for server names,
-  // for these reasons if the robot name is bigger than an arbitrary length, a hashed version is used instead
-  if (length > 70) {  // note: this threshold should be the same as in WbRobot.cpp
+  // limitations such as QLocalServer only accepting strings up to 91 characters long for server names
+  // on some platforms.
+  // Since the server name also contains the tmp path and that includes the user's username and,
+  // in the case of a Snap, the user's home directory, we need to limit the length of the encoded
+  // robot name based on the tmp path. If it is longer than that, then we compute a hashed version
+  // of the name and use as much of it as we can. If that would be less than 4 chars, we try to use
+  // 4 chars and hope we are on a platform where QLocalServer accepts longer names. 4 chars makes the
+  // chance of a name collision 1/65536.
+  // Note: It is critical that the same logic is used in WbRobot.cpp
+  int max_name_length = 91 - chars_used;
+  if (max_name_length < 4)
+    max_name_length = 4;
+  // Round down to the next multiple of 2 because it makes the code easier.
+  max_name_length = max_name_length / 2 * 2;
+  if (length > max_name_length) {
     char hash[21];
-    char *output = malloc(41);
+    char *output = malloc(max_name_length + 1);
     SHA1(hash, encoded_name, length);
     free(encoded_name);
-    for (size_t i = 0; i < 20; i++)
+    for (size_t i = 0; i < max_name_length / 2 && i < 20; i++)
       sprintf((output + (2 * i)), "%02x", hash[i] & 0xff);
     return output;
   }
@@ -1104,10 +1116,12 @@ static char *encode_robot_name(const char *robot_name) {
 }
 
 static char *compute_socket_filename(char *error_buffer) {
-  char *robot_name = encode_robot_name(wbu_system_getenv("WEBOTS_ROBOT_NAME"));
   const char *WEBOTS_INSTANCE_PATH = wbu_system_webots_instance_path(true);
+  char *robot_name = NULL;
+  const char *WEBOTS_ROBOT_NAME = wbu_system_getenv("WEBOTS_ROBOT_NAME");
   char *socket_filename;
-  if (robot_name && robot_name[0] && WEBOTS_INSTANCE_PATH && WEBOTS_INSTANCE_PATH[0]) {
+  if (WEBOTS_ROBOT_NAME && WEBOTS_ROBOT_NAME[0] && WEBOTS_INSTANCE_PATH && WEBOTS_INSTANCE_PATH[0]) {
+    robot_name = encode_robot_name(WEBOTS_ROBOT_NAME, strlen(WEBOTS_INSTANCE_PATH) + strlen("ipc//intern"));
 #ifndef _WIN32
     const int length = strlen(WEBOTS_INSTANCE_PATH) + strlen(robot_name) + 15;  // "%sintern/%s/socket"
     socket_filename = malloc(length);
@@ -1267,7 +1281,7 @@ static char *compute_socket_filename(char *error_buffer) {
 
   free(robot_name);
   const char *sub_string = strstr(&WEBOTS_CONTROLLER_URL[6], "/");
-  robot_name = encode_robot_name(sub_string ? sub_string + 1 : NULL);
+  robot_name = encode_robot_name(sub_string ? sub_string + 1 : NULL, strlen(ipc_folder) + strlen("//extern"));
   if (robot_name) {
 #ifndef _WIN32
     // socket file name is like: folder + robot_name + "/extern"
@@ -1370,7 +1384,7 @@ static void compute_remote_info(char **host, int *port, char **robot_name) {
   snprintf(*host, host_length, "%s", &WEBOTS_CONTROLLER_URL[6]);
   sscanf(url_suffix, ":%d", port);
   const char *rn = strstr(url_suffix, "/");
-  *robot_name = rn != NULL ? encode_robot_name(rn + 1) : NULL;
+  *robot_name = rn != NULL ? encode_robot_name(rn + 1, 0) : NULL;
 }
 
 int wb_robot_init() {  // API initialization
@@ -1462,11 +1476,11 @@ int wb_robot_init() {  // API initialization
         retry -= 5;
         fprintf(stderr, "%s, pending until loading is done...\n", error_message);
       } else if (socket_filename) {
+        fprintf(stderr,
+                "The specified robot (at %s) is not in the list of robots with <extern> controllers, retrying for another %d "
+                "seconds...\n",
+                socket_filename, 50 - retry);
         free(socket_filename);
-        fprintf(
-          stderr,
-          "The specified robot is not in the list of robots with <extern> controllers, retrying for another %d seconds...\n",
-          50 - retry);
       } else
         fprintf(stderr, "%s, retrying for another %d seconds...\n", error_message, 50 - retry);
     }

src/webots/gui/WbRenderingDeviceWindow.cpp

@@ -303,7 +303,7 @@ void WbRenderingDeviceWindow::renderNow() {
 
   if (!mContext) {
     mContext = new QOpenGLContext(this);
-    mContext->setFormat(requestedFormat());
+    mContext->setFormat(cMainOpenGLContext->format());
     mContext->setShareContext(cMainOpenGLContext);
     mContext->create();
   }

src/webots/nodes/WbRobot.cpp

@@ -59,6 +59,7 @@
 #include <QtCore/QTimer>
 #include <QtCore/QUrl>
 
+#include <algorithm>
 #include <limits>
 
 static QHash<int, int> createSpecialKeys() {
@@ -1093,12 +1094,25 @@ void WbRobot::dispatchAnswer(WbDataStream &stream, bool includeDevices) {
 }
 
 QString WbRobot::encodedName() const {
-  const QString encodedName = QUrl::toPercentEncoding(name());
+  QString encodedName = QUrl::toPercentEncoding(name());
   // the robot name is used to connect to the libController and in this process there are indirect
-  // limitations such as QLocalServer only accepting strings up to 106 characters for server names,
-  // for these reasons if the robot name is bigger than an arbitrary length, a hashed version is used instead
-  if (encodedName.length() > 70)  // note: this threshold should be the same as in robot.c
-    return QString(QCryptographicHash::hash(encodedName.toUtf8(), QCryptographicHash::Sha1).toHex());
+  // limitations such as QLocalServer only accepting strings up to 91 characters long for server names
+  // on some platforms.
+  // Since the server name also contains the tmp path and that includes the user's username and,
+  // in the case of a Snap, the user's home directory, we need to limit the length of the encoded
+  // robot name based on the tmp path. If it is longer than that, then we compute a hashed version
+  // of the name and use as much of it as we can. If that would be less than 4 chars, we try to use
+  // 4 chars and hope we are on a platform where QLocalServer accepts longer names. 4 chars makes the
+  // chance of a name collision 1/65536.
+  // Note: It is critical that the same logic is used in robot.c
+  const QString &tmpPath = WbStandardPaths::webotsTmpPath();
+  qsizetype maxNameLength = std::max((qsizetype)4, 91 - (tmpPath.length() + (qsizetype)strlen("ipc//extern")));
+  // Round down to the next multiple of 2 because it makes the code in robot.c easier.
+  maxNameLength = maxNameLength / 2 * 2;
+  if (encodedName.length() > maxNameLength) {
+    encodedName = QString(QCryptographicHash::hash(encodedName.toUtf8(), QCryptographicHash::Sha1).toHex());
+    encodedName.truncate(maxNameLength);
+  }
   return encodedName;
 }
 

src/webots/wren/WbWrenCamera.cpp

@@ -664,10 +664,13 @@ void WbWrenCamera::cleanup() {
   }
 
   WrTextureRtt *renderingTexture = wr_frame_buffer_get_output_texture(mResultFrameBuffer, 0);
-  WrTextureRtt *outputTexture = wr_frame_buffer_get_output_texture(mResultFrameBuffer, 1);
-  wr_frame_buffer_delete(mResultFrameBuffer);
   wr_texture_delete(WR_TEXTURE(renderingTexture));
+  // For some reason, deleting the outputTexture when cleaning the world causes crash on macos when closing the app.
+#ifndef __APPLE__
+  WrTextureRtt *outputTexture = wr_frame_buffer_get_output_texture(mResultFrameBuffer, 1);
   wr_texture_delete(WR_TEXTURE(outputTexture));
+#endif
+  wr_frame_buffer_delete(mResultFrameBuffer);
 
   wr_texture_delete(WR_TEXTURE(mNoiseMaskTexture));
   mNoiseMaskTexture = NULL;

tests/api/controllers/robot_urdf/robot_urdf.c

@@ -60,7 +60,9 @@ int main(int argc, char **argv) {
       break;
     usleep(100000);  // sleep for 100 ms
   };
-  ts_assert_int_not_equal(file_size, 0, "check_urdf command is missing");
+  ts_assert_int_not_equal(file_size, 0,
+                          "check_urdf command is missing. "
+                          "Install with 'apt install liburdfdom-tools' on Ubuntu or 'brew install urdfdom' on MacOS.");
   if ((urdf_check_status >> 8) != 127 && file_size > 0) {
     // `check_urdf` command is available
     // Verify output from `check_urdf`

tests/test_suite.py

@@ -350,20 +350,20 @@ def runGroupTest(groupName, firstSimulation, worldsCount, failures):
                                     webotsEmptyWorldPath], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
 atexit.register(subprocess.Popen.terminate, self=backgroundWebots)
 # Wait until we can actually connect to it, trying 10 times.
-with contextlib.closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock:
-    retries = 0
-    error = None
-    while retries < 10:
-        try:
+retries = 0
+error = None
+while retries < 10:
+    try:
+        with contextlib.closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock:
             sock.settimeout(1)
             sock.connect(("127.0.0.1", 1234))
             break
-        except socket.error as e:
-            error = e
-            retries += 1
-            time.sleep(1)
-    if retries == 10:
-        raise error
+    except socket.error as e:
+        error = e
+        retries += 1
+        time.sleep(1)
+if retries == 10:
+    raise error
 
 for groupName in testGroups:
     if groupName == 'cache':