protocols: printer: rename and reorg pieces

Moving some constants to a more public header. Renaming some
public and private variables to be more clear as to their purpose.
Things like that.

Signed-off-by: Kris Bahnsen <Kris@KBEmbedded.com>

protocols/printer/include/printer_proto.h

@@ -8,8 +8,23 @@
 
 #pragma once
 
+#define STATUS_LOWBATT		(1 << 7)
+#define STATUS_ERR		(1 << 6)
+#define STATUS_JAM		(1 << 5)
+#define STATUS_PKT_ERR		(1 << 4)
+#define STATUS_READY		(1 << 3)
+#define STATUS_FULL		(1 << 2)
+#define STATUS_PRINTING		(1 << 1)
+#define STATUS_CKSUM_ERR	(1 << 0)
+
+/* emulate printer's internal print receive buffer */
+#define TILE_SIZE		16 // 8x8 tile, 2bpp color
+#define WIDTH			20 // 20 tiles wide
+#define HEIGHT			18 // 18 tiles tall
+#define PRINT_FULL_SZ		5760 // (LINE_BUF_SZ * HEIGHT / 2)
+
 enum cb_reason {
-	reason_data,
+	reason_line_xfer,
 	reason_print,
 	reason_complete,
 };
@@ -30,9 +45,8 @@ struct gb_image {
 	/* TODO: Need to play with this more */
 	uint8_t exposure;
 
-	/* Always expected to be 160 px wide */
 	size_t data_sz;
-	uint8_t data[];
+	uint8_t data[PRINT_FULL_SZ];
 };
 
 /**
@@ -123,27 +137,4 @@ const GpioPin *printer_pin_get(void *printer_handle, gblink_bus_pins pin);
  */
 void printer_stop(void *printer_handle);
 
-/**
- * Allocate a gb_image structure for use outside of the printer instance
- *
- * Allocates a buffer of the maximum size that the Game Boy Printer can work as
- * a single image, 160x144 px, 20x18 tiles. Provided as a convenience function so
- * higher level applications don't need to know how big of a buffer to create.
- *
- * Useful for, e.g. copying a received image to and the marking that image as
- * "printed".
- *
- * Must be freed manually
- *
- * @returns Pointer to a struct gb_image
- */
-struct gb_image *printer_image_buffer_alloc(void);
-
-/**
- * Free a previously allocated gb_image structure
- *
- * @param image Pointer to gb_image struct
- */
-void printer_image_buffer_free(struct gb_image *image);
-
 #endif // PRINTER_PROTO_H

protocols/printer/printer_i.h

@@ -6,43 +6,59 @@
 
 #include <protocols/printer/include/printer_proto.h>
 
-#define PKT_START_L		0x88
-#define PKT_START_H		0x33
+#define START_L_BYTE		0x88
+#define START_H_BYTE		0x33
+#define ALIVE_BYTE		0x81
 
-#define PRINTER_ID		0x81
 #define CMD_INIT		0x01
 #define CMD_PRINT		0x02
 #define CMD_TRANSFER		0x10
 #define CMD_DATA		0x04
-#define CMD_BREAK		0x08 // ??
 #define CMD_STATUS		0x0f
 
-#define STATUS_LOWBATT		(1 << 7)
-#define STATUS_ERR		(1 << 6)
-#define STATUS_JAM		(1 << 5)
-#define STATUS_PKT_ERR		(1 << 4)
-#define STATUS_READY		(1 << 3)
-#define STATUS_FULL		(1 << 2)
-#define STATUS_PRINTING		(1 << 1)
-#define STATUS_CKSUM		(1 << 0)
-
-// Extra status bits used not used by the printer
-#define STATUS_PRINT_CMD	(1 << 8)
-#define STATUS_PRINTED		(1 << 9)
-
-/* emulate printer's internal print receive buffer */
-#define TILE_SIZE		16 // 8x8 tile, 2bpp color
-#define WIDTH			20 // 20 tiles wide
-#define HEIGHT			18 // 18 tiles tall
-#define PRINT_RECV_SZ		640 // (TILE_SIZE * WIDTH * 2)
-#define PRINT_FULL_SZ		5760 // (PRINT_RECV_SZ * HEIGHT / 2)
-#define TRANSFER_RECV_SZ	3584 // (16*16*14) // Image minus frame
-
-//Note that TRANSFER uses a locked size, 16x14 tiles, 16*16*14
-
-//GB seems to use 2 second busy timeout? I think that is a go to busy/printing within 2 seconds?
-//20 second print timeout
-
+/* These constants are private to the printer protocol */
+#define LINE_BUF_SZ		640 // (TILE_SIZE * WIDTH * 2)
+#define TRANSFER_SZ		3584 // (16*16*14) // Image minus frame
+
+/* NOTE:
+ * These numbers are empirically gathered from a few different games thus far.
+ * There are many notes floating around the internet of the GB Printer having
+ * a 100 ms limit between packets where it will reset. However, I've seen
+ * Pokemon Pinball wait 99.5 ms between packets after a print command which is
+ * a bit too close for comfort. As this code tracks timestamps _after_ each byte,
+ * that ends up just over 110 ms which trips the hard timeout and resets state.
+ * This often cofuses the hell out of games.
+ *
+ * Additionally, on the other end of the spectrum, Pokemon Gold absolutely uses
+ * the hard timeout to reset the printer between packets. It waits ~278 ms, when
+ * it could just send an init command.
+ *
+ * Even more silly, Pokemon Pinball has a fun quirk where if the print completes
+ * immediately (usually the Flipper will mark a print complete with a single
+ * packet turnaround), it asks for status a couple of times, then starts (presumably)
+ * another status packet, but the second byte in the transfer is stopped mid-byte.
+ * Between that point and the start of the next, real packet, is 30 ms, 32.6 ms
+ * if you go from end of last byte received to start of next, real packet.
+ *
+ * This means there is some "soft" timeout that the printer uses to reset a packet
+ * transfer in progress, but don't reset the whole printer state.
+ *
+ * There are wisps of some "unknown" bit timeout of 1.49 ms. But I've not yet
+ * seen that in action.
+ *
+ * As far as I know, no one has dumped and reverse engineered the ROM of the
+ * Game Boy Printer directly. I think all of the existing documentation was from
+ * reverse engineering the communication channel. Maybe someday I'll dump the
+ * GB Printer ROM and try to better understand all of it.
+ *
+ * Additionally, the gameboy camera seems to hint at a 2 second busy timeout, that
+ * is that the printer needs to return a printing status within 2 seconds of starting
+ * the print. There is also a hint of a 20 second print timeout, that is that the
+ * printer needs to return print complete within 20 seconds of starting the print.
+ * These two values are not really used in this library yet.
+ */
+#define HARD_TIMEOUT_US 125000
+#define SOFT_TIMEOUT_US 20000
 
 enum packet_state {
 	START_L,
@@ -51,25 +67,25 @@ enum packet_state {
 	COMPRESS,
 	LEN_L,
 	LEN_H,
-	COMMAND_DAT,
+	DATA,
 	CKSUM_L,
 	CKSUM_H,
 	ALIVE,
 	STATUS,
 };
 
-/* Does not need to care about start bytes */
 struct packet {
 	uint8_t cmd;
-	uint8_t compress;
+	bool compress;
 	uint16_t len; // This is stored in the flipper endianness, arrives LSB first from GB, unmodified in code
-	uint8_t recv_data[PRINT_RECV_SZ]; // 640 bytes, enough for two lines of tiles
+	uint8_t line_buf[LINE_BUF_SZ]; // 640 bytes, enough for two lines of tiles
 	uint16_t cksum; // This is stored in the flipper endianness, arrives LSB first from GB
+	uint8_t status;
 	
 	/* These are not part of the packet, but used by us */
 	uint16_t cksum_calc;
-	size_t recv_data_sz;
-	uint16_t status;
+	size_t line_buf_sz;
+	bool print_complete;
 	uint8_t zero_counter;
 	enum packet_state state;
 	uint32_t time;
@@ -91,7 +107,6 @@ struct printer_proto {
 
 	struct gb_image *image; // Details of the current image being sent/received
 
-
 	FuriThread *thread;
 };
 

protocols/printer/printer_proto.c

@@ -26,7 +26,7 @@ static int32_t printer_callback_thread(void *context)
 		if (flags & THREAD_FLAGS_EXIT)
 			break;
 		if (flags & THREAD_FLAGS_DATA)
-			printer->callback(printer->cb_context, printer->image, reason_data);
+			printer->callback(printer->cb_context, printer->image, reason_line_xfer);
 		if (flags & THREAD_FLAGS_PRINT)
 			printer->callback(printer->cb_context, printer->image, reason_print);
 	}
@@ -51,7 +51,7 @@ void *printer_alloc(void)
 	furi_thread_start(printer->thread);
 
 	printer->packet = malloc(sizeof(struct packet));
-	printer->image = printer_image_buffer_alloc();
+	printer->image = malloc(sizeof(struct gb_image));
 
 	printer->gblink_handle = gblink_alloc();
 
@@ -135,15 +135,3 @@ void printer_stop(void *printer_handle)
 	 * just stop?
 	 */
 }
-
-
-struct gb_image *printer_image_buffer_alloc(void)
-{
-	struct gb_image *image = malloc(sizeof(struct gb_image) + PRINT_FULL_SZ);
-	return image;
-}
-
-void printer_image_buffer_free(struct gb_image *image)
-{
-	free(image);
-}

protocols/printer/printer_receive.c

@@ -10,40 +10,6 @@
 
 #define TAG "printer_receive"
 
-/* NOTE:
- * These numbers are empirically gathered from a few different games thus far.
- * There are many notes floating around the internet of the GB Printer having
- * a 100 ms limit between packets where it will reset. However, I've seen
- * Pokemon Pinball wait 99.5 ms between packets after a print command which is
- * a bit too close for comfort. As this code tracks timestamps _after_ each byte,
- * that ends up just over 110 ms which trips the hard timeout and resets state.
- * This often cofuses the hell out of games.
- *
- * Additionally, on the other end of the spectrum, Pokemon Gold absolutely uses
- * the hard timeout to reset the printer between packets. It waits ~278 ms, when
- * it could just send an init command.
- *
- * Even more silly, Pokemon Pinball has a fun quirk where if the print completes
- * immediately (usually the Flipper will mark a print complete with a single
- * packet turnaround), it asks for status a couple of times, then starts (presumably)
- * another status packet, but the second byte in the transfer is stopped mid-byte.
- * Between that point and the start of the next, real packet, is 30 ms, 32.6 ms
- * if you go from end of last byte received to start of next, real packet.
- *
- * This means there is some "soft" timeout that the printer uses to reset a packet
- * transfer in progress, but don't reset the whole printer state.
- *
- * There are wisps of some "unknown" bit timeout of 1.49 ms. But I've not yet
- * seen that in action.
- *
- * As far as I know, no one has dumped and reverse engineered the ROM of the
- * Game Boy Printer directly. I think all of the existing documentation was from
- * reverse engineering the communication channel. Maybe someday I'll dump the
- * GB Printer ROM and try to better understand all of it.
- */
-#define HARD_TIMEOUT_US 125000
-#define SOFT_TIMEOUT_US 20000
-
 static void printer_reset(struct printer_proto *printer)
 {
 	/* Clear out the current packet data */
@@ -77,7 +43,7 @@ static void byte_callback(void *context, uint8_t val)
 
 	switch (packet->state) {
 	case START_L:
-		if (val == PKT_START_L) {
+		if (val == START_L_BYTE) {
 			packet->state = START_H;
 			packet->zero_counter = 0;
 		}
@@ -88,7 +54,7 @@ static void byte_callback(void *context, uint8_t val)
 		}
 		break;
 	case START_H:
-		if (val == PKT_START_H)
+		if (val == START_H_BYTE)
 			packet->state = COMMAND;
 		else
 			packet->state = START_L;
@@ -116,19 +82,19 @@ static void byte_callback(void *context, uint8_t val)
 		packet->len |= ((val & 0xff) << 8);
 		/* Override length for a TRANSFER */
 		if (packet->cmd == CMD_TRANSFER)
-			packet->len = TRANSFER_RECV_SZ;
+			packet->len = TRANSFER_SZ;
 
 		if (packet->len) {
-			packet->state = COMMAND_DAT;
+			packet->state = DATA;
 		} else {
 			packet->state = CKSUM_L;
 		}
 		break;
-	case COMMAND_DAT:
+	case DATA:
 		packet->cksum_calc += val;
-		packet->recv_data[packet->recv_data_sz] = val;
-		packet->recv_data_sz++;
-		if (packet->recv_data_sz == packet->len) 
+		packet->line_buf[packet->line_buf_sz] = val;
+		packet->line_buf_sz++;
+		if (packet->line_buf_sz == packet->len)
 			packet->state = CKSUM_L;
 		break;
 	case CKSUM_L:
@@ -139,11 +105,11 @@ static void byte_callback(void *context, uint8_t val)
 		packet->state = ALIVE;
 		packet->cksum |= ((val & 0xff) << 8);
 		if (packet->cksum != packet->cksum_calc)
-			packet->status |= STATUS_CKSUM;
+			packet->status |= STATUS_CKSUM_ERR;
 		// TRANSFER does not set checksum bytes
 		if (packet->cmd == CMD_TRANSFER)
-			packet->status &= ~STATUS_CKSUM;
-		data_out = PRINTER_ID;
+			packet->status &= ~STATUS_CKSUM_ERR;
+		data_out = ALIVE_BYTE;
 		break;
 	case ALIVE:
 		packet->state = STATUS;
@@ -158,10 +124,10 @@ static void byte_callback(void *context, uint8_t val)
 		case CMD_DATA:
 			if (printer->image->data_sz < PRINT_FULL_SZ) {
 				if ((printer->image->data_sz + packet->len) <= PRINT_FULL_SZ) {
-					memcpy((printer->image->data)+printer->image->data_sz, packet->recv_data, packet->len);
+					memcpy((printer->image->data)+printer->image->data_sz, packet->line_buf, packet->len);
 					printer->image->data_sz += packet->len;
 				} else {
-					memcpy((printer->image->data)+printer->image->data_sz, packet->recv_data, ((printer->image->data_sz + packet->len)) - PRINT_FULL_SZ);
+					memcpy((printer->image->data)+printer->image->data_sz, packet->line_buf, ((printer->image->data_sz + packet->len)) - PRINT_FULL_SZ);
 					printer->image->data_sz += (PRINT_FULL_SZ - (printer->image->data_sz + packet->len));
 					furi_assert(printer->image->data_sz <= PRINT_FULL_SZ);
 				}
@@ -178,10 +144,10 @@ static void byte_callback(void *context, uint8_t val)
 			 */
 		case CMD_PRINT:
 			/* TODO: Be able to memcpy these */
-			printer->image->num_sheets = packet->recv_data[0];
-			printer->image->margins = packet->recv_data[1];
-			printer->image->palette = packet->recv_data[2];
-			printer->image->exposure = packet->recv_data[3];
+			printer->image->num_sheets = packet->line_buf[0];
+			printer->image->margins = packet->line_buf[1];
+			printer->image->palette = packet->line_buf[2];
+			printer->image->exposure = packet->line_buf[3];
 			packet->status &= ~STATUS_READY;
 			packet->status |= (STATUS_PRINTING | STATUS_FULL);
 			furi_thread_flags_set(printer->thread, THREAD_FLAGS_PRINT);
@@ -189,14 +155,14 @@ static void byte_callback(void *context, uint8_t val)
 		case CMD_STATUS:
 			/* READY cleared on status request */
 			packet->status &= ~STATUS_READY;
-			if ((packet->status & STATUS_PRINTING) &&
-			    (packet->status & STATUS_PRINTED)) {
-				packet->status &= ~(STATUS_PRINTING | STATUS_PRINTED);
+			if ((packet->status & STATUS_PRINTING) && packet->print_complete) {
+				packet->status &= ~(STATUS_PRINTING);
+				packet->print_complete = false;
 				furi_thread_flags_set(printer->thread, THREAD_FLAGS_COMPLETE);
 			}
 		}
 
-		packet->recv_data_sz = 0;
+		packet->line_buf_sz = 0;
 		packet->cksum_calc = 0;
 
 
@@ -205,7 +171,7 @@ static void byte_callback(void *context, uint8_t val)
 		 * not printing -> printing -> not printing, etc.
 		 */
 		/* Do a callback here?
-		 * if so, I guess we should wait for callback completion before accepting more recv_data?
+		 * if so, I guess we should wait for callback completion before accepting more line_buf?
 		 * but that means the callback is in an interrupt context, which, is probably okay?
 		 */
 		/* XXX: TODO: NOTE: FIXME:
@@ -257,5 +223,5 @@ void printer_receive_print_complete(void *printer_handle)
 {
 	struct printer_proto *printer = printer_handle;
 
-	printer->packet->status |= STATUS_PRINTED;
+	printer->packet->print_complete = true;
 }