Issues 527, 528: AGW protocol compatible with NET/ROM.

Author: wb2osz

src/ax25_pad.c

@@ -2579,6 +2579,59 @@ int ax25_get_c2 (packet_t this_p)
 }
 
 
+/*------------------------------------------------------------------
+ *
+ * Function:	ax25_set_pid
+ *
+ * Purpose:	Set protocol ID in packet.
+ *
+ * Inputs:	this_p	- pointer to packet object.
+ *
+ *		pid - usually 0xF0 for APRS or 0xCF for NET/ROM.
+ *
+ * AX.25:	"The Protocol Identifier (PID) field appears in information
+ *		 frames (I and UI) only. It identifies which kind of
+ *		 Layer 3 protocol, if any, is in use."
+ *
+ *------------------------------------------------------------------*/
+
+void ax25_set_pid (packet_t this_p, int pid)
+{
+	assert (this_p->magic1 == MAGIC);
+	assert (this_p->magic2 == MAGIC);
+
+	// Some applications set this to 0 which is an error.
+	// Change 0 to 0xF0 meaning no layer 3 protocol.
+
+	if (pid == 0) {
+	  pid = AX25_PID_NO_LAYER_3;
+	}
+
+	// Sanity check: is it I or UI frame?
+
+	if (this_p->frame_len == 0) return;
+
+	ax25_frame_type_t frame_type;
+	cmdres_t cr;			// command or response.
+	char description[64];
+	int pf;				// Poll/Final.
+	int nr, ns;			// Sequence numbers.
+
+	frame_type = ax25_frame_type (this_p, &cr, description, &pf, &nr, &ns);
+
+	if (frame_type != frame_type_I && frame_type != frame_type_U_UI) {
+          text_color_set(DW_COLOR_ERROR);
+          dw_printf ("ax25_set_pid(0x%2x): Packet type is not I or UI.\n", pid);
+	  return;
+	}
+
+	// TODO: handle 2 control byte case.
+	if (this_p->num_addr >= 2) {
+	  this_p->frame_data[ax25_get_pid_offset(this_p)] = pid;
+	}
+}
+
+
 /*------------------------------------------------------------------
  *
  * Function:	ax25_get_pid

src/ax25_pad.h

@@ -66,6 +66,7 @@
 #define AX25_UI_FRAME 3		/* Control field value. */
 
 #define AX25_PID_NO_LAYER_3 0xf0		/* protocol ID used for APRS */
+#define AX25_PID_NETROM 0xcf			/* protocol ID used for NET/ROM */
 #define AX25_PID_SEGMENTATION_FRAGMENT 0x08
 #define AX25_PID_ESCAPE_CHARACTER 0xff
 
@@ -427,6 +428,7 @@ extern int ax25_is_null_frame (packet_t this_p);
 extern int ax25_get_control (packet_t this_p); 
 extern int ax25_get_c2 (packet_t this_p); 
 
+extern void ax25_set_pid (packet_t this_p, int pid);
 extern int ax25_get_pid (packet_t this_p);
 
 extern int ax25_get_frame_len (packet_t this_p);

src/server.c

@@ -379,7 +379,7 @@ static void debug_print (fromto_t fromto, int client, struct agwpe_s *pmsg, int
 	      case 'C': strlcpy (datakind, "AX.25 Connection Received",			sizeof(datakind)); break;
 	      case 'D': strlcpy (datakind, "Connected AX.25 Data",			sizeof(datakind)); break;
 	      case 'd': strlcpy (datakind, "Disconnected",				sizeof(datakind)); break;
-	      case 'M': strlcpy (datakind, "Monitored Connected Information",		sizeof(datakind)); break;
+	      case 'I': strlcpy (datakind, "Monitored Connected Information",		sizeof(datakind)); break;
 	      case 'S': strlcpy (datakind, "Monitored Supervisory Information",		sizeof(datakind)); break;
 	      case 'U': strlcpy (datakind, "Monitored Unproto Information",		sizeof(datakind)); break;
 	      case 'T': strlcpy (datakind, "Monitoring Own Information",		sizeof(datakind)); break;
@@ -1717,6 +1717,7 @@ static THREAD_F cmd_listen_thread (void *arg)
 
 		packet_t pp;
 
+		int pid = cmd.hdr.pid;
 	      	strlcpy (stemp, cmd.hdr.call_from, sizeof(stemp));
 	      	strlcat (stemp, ">", sizeof(stemp));
 	      	strlcat (stemp, cmd.hdr.call_to, sizeof(stemp));
@@ -1730,33 +1731,41 @@ static THREAD_F cmd_listen_thread (void *arg)
 		  strlcat (stemp, p, sizeof(stemp));
 		  p += 10;
 	        }
+	        // At this point, p now points to info part after digipeaters.
+
+	        // Issue 527: NET/ROM routing broadcasts are binary info so we can't treat as string.
+	        // Originally, I just appended the information part.
+		// That was fine until NET/ROM, with binary data, came along.
+		// Now we set the information field after creating the packet object.
+
 		strlcat (stemp, ":", sizeof(stemp));
-		strlcat (stemp, p, sizeof(stemp));
+		strlcat (stemp, " ", sizeof(stemp));
 
 	        //text_color_set(DW_COLOR_DEBUG);
 		//dw_printf ("Transmit '%s'\n", stemp);
 
 		pp = ax25_from_text (stemp, 1);
 
-
 		if (pp == NULL) {
 	          text_color_set(DW_COLOR_ERROR);
 		  dw_printf ("Failed to create frame from AGW 'V' message.\n");
+	          break;
 		}
-		else {
 
-		  /* This goes into the low priority queue because it is an original. */
+	        ax25_set_info (pp, (unsigned char*)p, data_len - ndigi * 10);
+	        // Issue 527: NET/ROM routing broadcasts use PID 0xCF which was not preserved here.
+	        ax25_set_pid (pp, pid);
 
-		  /* Note that the protocol has no way to set the "has been used" */
-		  /* bits in the digipeater fields. */
+		/* This goes into the low priority queue because it is an original. */
 
-		  /* This explains why the digipeating option is grayed out in */
-		  /* xastir when using the AGW interface.  */
-		  /* The current version uses only the 'V' message, not 'K' for transmitting. */
+		/* Note that the protocol has no way to set the "has been used" */
+		/* bits in the digipeater fields. */
 
-		  tq_append (cmd.hdr.portx, TQ_PRIO_1_LO, pp);
+		/* This explains why the digipeating option is grayed out in */
+		/* xastir when using the AGW interface.  */
+		/* The current version uses only the 'V' message, not 'K' for transmitting. */
 
-		}
+		tq_append (cmd.hdr.portx, TQ_PRIO_1_LO, pp);
 	      }
 
 	      break;
@@ -1890,15 +1899,14 @@ static THREAD_F cmd_listen_thread (void *arg)
 	          unsigned char num_digi;	/* Expect to be in range 1 to 7.  Why not up to 8? */
 		  char dcall[7][10];
 	        } 
-#if 1
+
 		// October 2017.  gcc ??? complained:
 		//     warning: dereferencing pointer 'v' does break strict-aliasing rules
 		// Try adding this attribute to get rid of the warning.
 	        // If this upsets your compiler, take it out.
 	        // Let me know.  Maybe we could put in a compiler version check here.
 
 	           __attribute__((__may_alias__))
-#endif
 	                              *v = (struct via_info *)cmd.data;
 
 	        char callsigns[AX25_MAX_ADDRS][AX25_MAX_ADDR_LEN];
@@ -2007,41 +2015,37 @@ static THREAD_F cmd_listen_thread (void *arg)
 	      {
 
 		int pid = cmd.hdr.pid;
-		(void)(pid);
-			/* The AGW protocol spec says, */
-			/* "AX.25 PID 0x00 or 0xF0 for AX.25 0xCF NETROM and others" */
-
-			/* BUG: In theory, the AX.25 PID octet should be set from this. */
-			/* All examples seen (above) have 0. */
-			/* The AX.25 protocol spec doesn't list 0 as a valid value. */
-			/* We always send 0xf0, meaning no layer 3. */
-			/* Maybe we should have an ax25_set_pid function for cases when */
-			/* it is neither 0 nor 0xf0. */
-
 	      	char stemp[AX25_MAX_PACKET_LEN];
-		packet_t pp;
 
 	      	strlcpy (stemp, cmd.hdr.call_from, sizeof(stemp));
 	      	strlcat (stemp, ">", sizeof(stemp));
 	      	strlcat (stemp, cmd.hdr.call_to, sizeof(stemp));
 
 		cmd.data[data_len] = '\0';
 
+	        // Issue 527: NET/ROM routing broadcasts are binary info so we can't treat as string.
+	        // Originally, I just appended the information part as a text string.
+		// That was fine until NET/ROM, with binary data, came along.
+		// Now we set the information field after creating the packet object.
+
 		strlcat (stemp, ":", sizeof(stemp));
-		strlcat (stemp, cmd.data, sizeof(stemp));
+		strlcat (stemp, " ", sizeof(stemp));
 
 	        //text_color_set(DW_COLOR_DEBUG);
 		//dw_printf ("Transmit '%s'\n", stemp);
 
-		pp = ax25_from_text (stemp, 1);
+		packet_t pp = ax25_from_text (stemp, 1);
 
 		if (pp == NULL) {
 	          text_color_set(DW_COLOR_ERROR);
 		  dw_printf ("Failed to create frame from AGW 'M' message.\n");
 		}
-		else {
-		  tq_append (cmd.hdr.portx, TQ_PRIO_1_LO, pp);
-		}
+
+	        ax25_set_info (pp, (unsigned char*)cmd.data, data_len);
+	        // Issue 527: NET/ROM routing broadcasts use PID 0xCF which was not preserved here.
+	        ax25_set_pid (pp, pid);
+
+		tq_append (cmd.hdr.portx, TQ_PRIO_1_LO, pp);
 	      }
 	      break;