Merge remote-tracking branch 'origin/2.8' into 2.9

* origin/2.8:
  fix a race in communication between Task and Motion
  motion: fix indentation & comment, no behavior change

Author: SebKuzminsky

src/emc/motion-logger/motion-logger.c

@@ -284,12 +284,11 @@ int main(int argc, char* argv[]) {
     init_comm_buffers();
 
     while (1) {
-        if (c->commandNum != c->tail) {
-            // "split read"
-            continue;
-        }
+        rtapi_mutex_get(&emcmotStruct->command_mutex);
+
         if (c->commandNum == emcmotStatus->commandNumEcho) {
             // nothing new
+            rtapi_mutex_give(&emcmotStruct->command_mutex);
             maybe_reopen_logfile();
             usleep(10 * 1000);
             continue;
@@ -682,6 +681,8 @@ int main(int argc, char* argv[]) {
         emcmotStatus->commandNumEcho = c->commandNum;
         emcmotStatus->commandStatus = EMCMOT_COMMAND_OK;
         emcmotStatus->tail = emcmotStatus->head;
+
+        rtapi_mutex_give(&emcmotStruct->command_mutex);
     }
 
     return 0;

src/emc/motion/command.c

@@ -57,10 +57,12 @@
 #include <float.h>
 #include "posemath.h"
 #include "rtapi.h"
+#include "rtapi_mutex.h"
 #include "hal.h"
 #include "motion.h"
 #include "tp.h"
 #include "mot_priv.h"
+#include "motion_struct.h"
 #include "rtapi_math.h"
 #include "motion_types.h"
 #include "homing.h"
@@ -374,11 +376,14 @@ STATIC int is_feed_type(int motion_type)
     }
 }
 
+
 /*
   emcmotCommandHandler() is called each main cycle to read the
   shared memory buffer
+
+  This function runs with the emcmotCommand struct locked.
   */
-void emcmotCommandHandler(void *arg, long servo_period)
+void emcmotCommandHandler_locked(void *arg, long servo_period)
 {
     int joint_num, spindle_num;
     int n,s0,s1;
@@ -389,11 +394,6 @@ void emcmotCommandHandler(void *arg, long servo_period)
     int abort = 0;
     char* emsg = "";
 
-    /* check for split read */
-    if (emcmotCommand->head != emcmotCommand->tail) {
-	emcmotInternal->split++;
-	return;			/* not really an error */
-    }
     if (emcmotCommand->commandNum != emcmotStatus->commandNumEcho) {
 	/* increment head count-- we'll be modifying emcmotStatus */
 	emcmotStatus->head++;
@@ -1917,3 +1917,15 @@ void emcmotCommandHandler(void *arg, long servo_period)
 
     return;
 }
+
+
+void emcmotCommandHandler(void *arg, long servo_period) {
+    if (rtapi_mutex_try(&emcmotStruct->command_mutex) != 0) {
+        // Failed to take the mutex, because it is held by Task.
+        // This means Task is in the process of updating the command.
+        // Give up for now, and try again on the next invocation.
+        return;
+    }
+    emcmotCommandHandler_locked(arg, servo_period);
+    rtapi_mutex_give(&emcmotStruct->command_mutex);
+}

src/emc/motion/motion.c

@@ -855,10 +855,8 @@ static int init_comm_buffers(void)
     emcmotErrorInit(emcmotError);
 
     /* init command struct */
-    emcmotCommand->head = 0;
     emcmotCommand->command = 0;
     emcmotCommand->commandNum = 0;
-    emcmotCommand->tail = 0;
 
     /* init status struct */
     emcmotStatus->head = 0;

src/emc/motion/motion.h

@@ -206,7 +206,6 @@ extern "C" {
    memory, and all commands from higher level code come thru it.
 */
     typedef struct emcmot_command_t {
-	unsigned char head;	/* flag count for mutex detect */
 	cmd_code_t command;	/* command code (enum) */
 	int commandNum;		/* increment this for new command */
 	double motor_offset;    /* offset from joint to motor position */
@@ -260,7 +259,6 @@ extern "C" {
     char    direction;      /* CANON_DIRECTION flag for spindle orient */
     double  timeout;        /* of wait for spindle orient to complete */
     unsigned char wait_for_spindle_at_speed; // EMCMOT_SPINDLE_ON now carries this, for next feed move
-    unsigned char tail;	/* flag count for mutex detect */
     int arcBlendOptDepth;
     int arcBlendEnable;
     int arcBlendFallbackEnable;

src/emc/motion/motion_struct.h

@@ -12,10 +12,14 @@
 #ifndef MOTION_STRUCT_H
 #define MOTION_STRUCT_H
 
+#include <rtapi_mutex.h>
+
+
 /* big comm structure, for upper memory */
     typedef struct emcmot_struct_t {
-	struct emcmot_command_t command;	/* struct used to pass commands/data
-					   to the RT module from usr space */
+        rtapi_mutex_t command_mutex;  // Used to protect access to `command`.
+        struct emcmot_command_t command;   /* struct used to pass commands/data from Task to Motion */
+
 	struct emcmot_status_t status;	/* Struct used to store RT status */
 	struct emcmot_config_t config;	/* Struct used to store RT config */
 	struct emcmot_error_t error;	/* ring buffer for error messages */

src/emc/motion/usrmotintf.cc

@@ -75,24 +75,26 @@ int usrmotWriteEmcmotCommand(emcmot_command_t * c)
 {
     emcmot_status_t s;
     static int commandNum = 0;
-    static unsigned char headCount = 0;
     double end;
 
     if (!MOTION_ID_VALID(c->id)) {
         rcs_print("USRMOT: ERROR: invalid motion id: %d\n",c->id);
 	return EMCMOT_COMM_INVALID_MOTION_ID;
     }
-    c->head = ++headCount;
-    c->tail = c->head;
+
     c->commandNum = ++commandNum;
 
     /* check for mapped mem still around */
     if (0 == emcmotCommand) {
         rcs_print("USRMOT: ERROR: can't connect to shared memory\n");
 	return EMCMOT_COMM_ERROR_CONNECT;
     }
+
     /* copy entire command structure to shared memory */
+    rtapi_mutex_get(&emcmotStruct->command_mutex);
     *emcmotCommand = *c;
+    rtapi_mutex_give(&emcmotStruct->command_mutex);
+
     /* poll for receipt of command */
     /* set timeout for comm failure, now + timeout */
     end = etime() + EMCMOT_COMM_TIMEOUT;