clean up message type architecture

5 files changed, 74 insertions, 145 deletions

diff --git a/lib/pbdrv/pb-msg.c b/lib/pbdrv/pb-msg.c
index 8bf96ef..695a696 100644
--- a/lib/pbdrv/pb-msg.c
+++ b/lib/pbdrv/pb-msg.c
@@ -36,14 +36,16 @@ void pb_msg_free(pb_msg_t * msg) {
 }
 
 pb_buf_t pb_msg_write_req_magic() {
-	pb_cmd_req_magic_t content = {
+	pb_cmd_magic_t content = {
 		.magic = (char *) &pb_cmd_magic_req[0],
 		._magic_size = sizeof(pb_cmd_magic_req),
 	};
 	pb_msg_t msg_write = {
-		.type = PB_CMD_REQ_MAGIC,
+		.type = PB_CMD_MAGIC,
+		.action = PB_ACTION_REQ,
 		.sender = 0,
 		.msg = &content,
 	};
 	return pb_msg_write(&msg_write);
 }
+
diff --git a/lib/pbdrv/pb-serial.c b/lib/pbdrv/pb-serial.c
index 7eb738b..e9806e0 100644
--- a/lib/pbdrv/pb-serial.c
+++ b/lib/pbdrv/pb-serial.c
@@ -4,23 +4,29 @@
 #include "pb-types.h"
 
 void pb_ser_w(mpack_writer_t * writer, const pb_msg_t * msg) {
-	pb_ser_w_msg_header(writer, msg);
+	mpack_write_u8(writer, msg->type);
+	mpack_write_u8(writer, msg->action);
+	// TODO: if pb is compiled under the pb-mod target, place
+	// PB_MOD_ADDR in this field
+	mpack_write_u16(writer, msg->sender);
+
 	if (msg->msg == NULL) return;
 	switch (msg->type) {
-		case PB_CMD_REQ_READ: return pb_ser_w_cmd_req_read(writer, msg);
-		case PB_CMD_RES_READ: return pb_ser_w_cmd_res_read(writer, msg);
-		case PB_CMD_REQ_MAGIC: return pb_ser_w_cmd_req_magic(writer, msg);
-		case PB_CMD_RES_MAGIC: return pb_ser_w_cmd_res_magic(writer, msg);
+		case PB_CMD_PROP: return pb_ser_w_cmd_prop(writer, msg);
+		// case PB_CMD_STATE: return pb_ser_w_cmd_state(writer, msg);
+		case PB_CMD_MAGIC: return pb_ser_w_cmd_magic(writer, msg);
 		default: break;
 	}
 }
 void pb_ser_r(mpack_reader_t * reader, pb_msg_t * msg) {
-	pb_ser_r_msg_header(reader, msg);
+	msg->type = mpack_expect_u8(reader);
+	msg->action = mpack_expect_u8(reader);
+	msg->sender = mpack_expect_u16(reader);
+
 	switch (msg->type) {
-		case PB_CMD_REQ_READ: return pb_ser_r_cmd_req_read(reader, msg);
-		case PB_CMD_RES_READ: return pb_ser_r_cmd_res_read(reader, msg);
-		case PB_CMD_REQ_MAGIC: return pb_ser_r_cmd_req_magic(reader, msg);
-		case PB_CMD_RES_MAGIC: return pb_ser_r_cmd_res_magic(reader, msg);
+		case PB_CMD_PROP: return pb_ser_r_cmd_prop(reader, msg);
+		// case PB_CMD_STATE: return pb_ser_r_cmd_state(reader, msg);
+		case PB_CMD_MAGIC: return pb_ser_r_cmd_magic(reader, msg);
 		default: break;
 	}
 }
@@ -28,59 +34,30 @@ void pb_ser_free(pb_msg_t * msg) {
 	if (msg == NULL) return;
 
 	switch (msg->type) {
-		case PB_CMD_REQ_READ: return pb_ser_free_cmd_req_read(msg);
-		case PB_CMD_RES_READ: return pb_ser_free_cmd_res_read(msg);
-		case PB_CMD_REQ_MAGIC: return pb_ser_free_cmd_req_magic(msg);
-		case PB_CMD_RES_MAGIC: return pb_ser_free_cmd_res_magic(msg);
+		case PB_CMD_PROP: return pb_ser_free_cmd_prop(msg);
+		// case PB_CMD_STATE: return pb_ser_free_cmd_state(msg);
+		case PB_CMD_MAGIC: return pb_ser_free_cmd_magic(msg);
 		default: break;
 	}
-	pb_ser_free_msg_header(msg);
 }
 
-void pb_ser_w_msg_header(mpack_writer_t * writer, const pb_msg_t * msg) {
-	mpack_write_u8(writer, msg->type);
-	// TODO: if pb is compiled under the pb-mod target, place
-	// PB_MOD_ADDR in this field
-	mpack_write_u16(writer, msg->sender);
-}
-void pb_ser_r_msg_header(mpack_reader_t * reader, pb_msg_t * msg) {
-	msg->type = mpack_expect_u8(reader);
-	msg->sender = mpack_expect_u16(reader);
-}
-void pb_ser_free_msg_header(pb_msg_t * msg) { }
-
-void pb_ser_w_cmd_req_read(mpack_writer_t * writer, const pb_msg_t * _msg) {
-	pb_cmd_req_read_t * msg = _msg->msg;
-	mpack_write_u8(writer, msg->propid);
-}
-void pb_ser_r_cmd_req_read(mpack_reader_t * reader, pb_msg_t * _msg) {
-	pb_cmd_req_read_t * msg = _msg->msg = malloc(sizeof(pb_cmd_req_read_t));
-	msg->propid = mpack_expect_u8(reader);
-}
-void pb_ser_free_cmd_req_read(pb_msg_t * _msg) {
-	if (_msg->msg != NULL) {
-		free(_msg->msg);
-		_msg->msg = NULL;
-	}
-}
-
-void pb_ser_w_cmd_res_read(mpack_writer_t * writer, const pb_msg_t * _msg) {
-	pb_cmd_res_read_t * msg = _msg->msg;
+void pb_ser_w_cmd_prop(mpack_writer_t * writer, const pb_msg_t * _msg) {
+	pb_cmd_prop_t * msg = _msg->msg;
 
 	mpack_write_u8(writer, msg->propid);
 	mpack_write_bin(writer, (char *) msg->value, msg->_value_size);
 }
-void pb_ser_r_cmd_res_read(mpack_reader_t * reader, pb_msg_t * _msg) {
-	pb_cmd_res_read_t * msg = _msg->msg = malloc(sizeof(pb_cmd_res_read_t));
+void pb_ser_r_cmd_prop(mpack_reader_t * reader, pb_msg_t * _msg) {
+	pb_cmd_prop_t * msg = _msg->msg = malloc(sizeof(pb_cmd_prop_t));
 
 	msg->propid = mpack_expect_u8(reader);
 	msg->_value_size = mpack_expect_bin(reader);
 	msg->value = (uint8_t *) mpack_read_bytes_alloc(reader, msg->_value_size);
 	mpack_done_bin(reader);
 }
-void pb_ser_free_cmd_res_read(pb_msg_t * _msg) {
+void pb_ser_free_cmd_prop(pb_msg_t * _msg) {
 	if (_msg->msg != NULL) {
-		pb_cmd_res_read_t * msg = _msg->msg;
+		pb_cmd_prop_t * msg = _msg->msg;
 		if (msg->value != NULL) {
 			MPACK_FREE(msg->value);
 			msg->value = NULL;
@@ -90,45 +67,21 @@ void pb_ser_free_cmd_res_read(pb_msg_t * _msg) {
 	}
 }
 
-void pb_ser_w_cmd_req_magic(mpack_writer_t * writer, const pb_msg_t * _msg) {
-	pb_cmd_req_magic_t * msg = _msg->msg;
-
-	mpack_write_bin(writer, (char *) msg->magic, msg->_magic_size);
-}
-void pb_ser_r_cmd_req_magic(mpack_reader_t * reader, pb_msg_t * _msg) {
-	pb_cmd_req_magic_t * msg = _msg->msg = malloc(sizeof(pb_cmd_req_magic_t));
-
-	msg->_magic_size = mpack_expect_bin(reader);
-	msg->magic = mpack_read_bytes_alloc(reader, msg->_magic_size);
-	mpack_done_bin(reader);
-}
-void pb_ser_free_cmd_req_magic(pb_msg_t * _msg) {
-	if (_msg->msg != NULL) {
-		pb_cmd_req_magic_t * msg = _msg->msg;
-		if (msg->magic != NULL) {
-			MPACK_FREE(msg->magic);
-			msg->magic = NULL;
-		}
-		free(_msg->msg);
-		_msg->msg = NULL;
-	}
-}
-
-void pb_ser_w_cmd_res_magic(mpack_writer_t * writer, const pb_msg_t * _msg) {
-	pb_cmd_res_magic_t * msg = _msg->msg;
+void pb_ser_w_cmd_magic(mpack_writer_t * writer, const pb_msg_t * _msg) {
+	pb_cmd_magic_t * msg = _msg->msg;
 
 	mpack_write_bin(writer, (char *) msg->magic, msg->_magic_size);
 }
-void pb_ser_r_cmd_res_magic(mpack_reader_t * reader, pb_msg_t * _msg) {
-	pb_cmd_res_magic_t * msg = _msg->msg = malloc(sizeof(pb_cmd_res_magic_t));
+void pb_ser_r_cmd_magic(mpack_reader_t * reader, pb_msg_t * _msg) {
+	pb_cmd_magic_t * msg = _msg->msg = malloc(sizeof(pb_cmd_magic_t));
 
 	msg->_magic_size = mpack_expect_bin(reader);
 	msg->magic = mpack_read_bytes_alloc(reader, msg->_magic_size);
 	mpack_done_bin(reader);
 }
-void pb_ser_free_cmd_res_magic(pb_msg_t * _msg) {
+void pb_ser_free_cmd_magic(pb_msg_t * _msg) {
 	if (_msg->msg != NULL) {
-		pb_cmd_res_magic_t * msg = _msg->msg;
+		pb_cmd_magic_t * msg = _msg->msg;
 		if (msg->magic != NULL) {
 			MPACK_FREE(msg->magic);
 			msg->magic = NULL;
diff --git a/lib/pbdrv/pb-serial.h b/lib/pbdrv/pb-serial.h
index 22031dc..d3d0007 100644
--- a/lib/pbdrv/pb-serial.h
+++ b/lib/pbdrv/pb-serial.h
@@ -22,15 +22,9 @@ pb_ser_r_t pb_ser_r;
 typedef void pb_ser_free_t(pb_msg_t * msg);
 pb_ser_free_t pb_ser_free;
 
-__pb_cmd(msg_header)
-__pb_cmd(cmd_req_read)
-__pb_cmd(cmd_res_read)
-__pb_cmd(cmd_req_write)
-__pb_cmd(cmd_req_state)
-__pb_cmd(cmd_res_state)
-__pb_cmd(cmd_req_set_state)
-__pb_cmd(cmd_req_magic)
-__pb_cmd(cmd_res_magic)
+__pb_cmd(cmd_prop)
+__pb_cmd(cmd_state)
+__pb_cmd(cmd_magic)
 
 #ifdef __cplusplus
 }
diff --git a/lib/pbdrv/pb-types.h b/lib/pbdrv/pb-types.h
index 53060ae..e3280da 100644
--- a/lib/pbdrv/pb-types.h
+++ b/lib/pbdrv/pb-types.h
@@ -1,5 +1,6 @@
 #pragma once
 
+#include <stdbool.h>
 #include <stdint.h>
 #include <stddef.h>
 
@@ -20,21 +21,19 @@ typedef uint16_t i2c_addr_t;
 
 //! puzzle bus command types
 enum pb_cmd_id {
-	PB_CMD_REQ_READ, //!< request a puzzle module property
-	PB_CMD_RES_READ, //!< respond to a puzzle module property request
-	PB_CMD_REQ_WRITE, //!< request to write a puzzle module property
-	PB_CMD_REQ_STATE, //!< request global state
-	PB_CMD_RES_STATE, //!< respond to a global state request
-	PB_CMD_REQ_SET_STATE, //!< request to overwrite module global state
-	PB_CMD_REQ_MAGIC, //!< magic request
-	PB_CMD_RES_MAGIC, //!< magic response
+	PB_CMD_PROP, //!< puzzle module property
+	PB_CMD_STATE, //!< global state
+	PB_CMD_MAGIC, //!< magic (handshake)
 };
 typedef enum pb_cmd_id pb_cmd_id_t;
 
-//! magic sent from main controller to puzzle module
-static const char pb_cmd_magic_req[] = { 0x70, 0x75, 0x7a, 0x62, 0x75, 0x73 };
-//! magic reply from puzzle module back to main controller
-static const char pb_cmd_magic_res[] = { 0x67, 0x61, 0x6d, 0x69, 0x6e, 0x67 };
+//! puzzle bus command action types
+enum pb_action {
+	PB_ACTION_REQ, //!< request
+	PB_ACTION_RES, //!< response
+	PB_ACTION_SET, //!< (over)write
+};
+typedef enum pb_action pb_action_t;
 
 //! puzzle bus global states
 enum pb_global_state {
@@ -45,58 +44,36 @@ enum pb_global_state {
 };
 typedef enum pb_global_state pb_global_state_t;
 
+//! magic sent from main controller to puzzle module
+static const char pb_cmd_magic_req[] = { 0x70, 0x75, 0x7a, 0x62, 0x75, 0x73 };
+//! magic reply from puzzle module back to main controller
+static const char pb_cmd_magic_res[] = { 0x67, 0x61, 0x6d, 0x69, 0x6e, 0x67 };
+
 //! puzzle bus message header (shared by all commands)
 typedef struct {
 	pb_cmd_id_t type; //!< command type
+	pb_action_t action; //!< command action
 	i2c_addr_t sender; //!< i2c address of sender
 	void * msg; //!< remaining message (type dependant)
 } pb_msg_t;
 
-//! PB_CMD_REQ_READ data
+//! PB_CMD_PROP data
 typedef struct {
-	uint8_t propid; //!< state property id to return
-} pb_cmd_req_read_t;
+	uint8_t propid; //!< id of state property
+	uint8_t * value; //!< new or current value
+	size_t _value_size; //!< [META] size of \p value
+} pb_cmd_prop_t;
 
-//! PB_CMD_RES_READ data
+//! PB_CMD_STATE data
 typedef struct {
-	uint8_t propid; //!< id of returned state property
-	uint8_t * value;
-	size_t _value_size;
-} pb_cmd_res_read_t;
-
-//! PB_CMD_REQ_WRITE data
-typedef struct {
-	uint8_t propid; //!< state property id to write
-	uint8_t * value; //!< new value of property
-	size_t _value_size;
-} pb_cmd_req_write_t;
-
-//! PB_CMD_REQ_STATE data
-typedef struct {
-	pb_global_state_t state; //!< global state of sender
-} pb_cmd_req_state_t;
-
-//! PB_CMD_RES_STATE data
-typedef struct {
-	pb_global_state_t state; //!< global state of sender
-} pb_cmd_res_state_t;
-
-//! PB_CMD_REQ_SET_STATE data
-typedef struct {
-	pb_global_state_t state; //!< new global state
-} pb_cmd_req_set_state_t;
-
-//! PB_CMD_REQ_MAGIC data
-typedef struct {
-	char * magic; //!< magic value
-	size_t _magic_size;
-} pb_cmd_req_magic_t;
+	pb_global_state_t state; //!< global state
+} pb_cmd_state_t;
 
-//! PB_CMD_RES_MAGIC data
+//! PB_CMD_MAGIC data
 typedef struct {
 	char * magic; //!< magic value
-	size_t _magic_size;
-} pb_cmd_res_magic_t;
+	size_t _magic_size; //!< [META] size of \p magic
+} pb_cmd_magic_t;
 
 #ifdef __cplusplus
 }
diff --git a/test/pbdrv/msg.cpp b/test/pbdrv/msg.cpp
index 38df8f4..afc548e 100644
--- a/test/pbdrv/msg.cpp
+++ b/test/pbdrv/msg.cpp
@@ -3,11 +3,12 @@
 #include "pb-msg.h"
 
 TEST(pb_msg_rw, cmd_req_read) {
-	pb_cmd_req_read_t content = {
+	pb_cmd_prop_t content = {
 		.propid = 2,
 	};
 	pb_msg_t msg_write = {
-		.type = PB_CMD_REQ_READ,
+		.type = PB_CMD_PROP,
+		.action = PB_ACTION_REQ,
 		.sender = 0xff,
 		.msg = &content,
 	};
@@ -23,7 +24,9 @@ TEST(pb_msg_rw, cmd_req_read) {
 
 	EXPECT_EQ(msg_write.type, msg_read->type);
 	EXPECT_EQ(msg_write.sender, msg_read->sender);
-	EXPECT_EQ(((pb_cmd_req_read_t *) msg_write.msg)->propid, ((pb_cmd_req_read_t *) msg_read->msg)->propid);
+	EXPECT_NE(nullptr, msg_read->msg);
+	pb_cmd_prop_t * prop = (pb_cmd_prop_t *) msg_read->msg;
+	EXPECT_EQ(prop->propid, content.propid);
 
 	pb_msg_free(msg_read);
 }
@@ -39,10 +42,10 @@ TEST(pb_msg_rw, cmd_req_magic) {
 
 	ASSERT_EQ(buf.data, nullptr);
 
-	EXPECT_EQ(msg_read->type, PB_CMD_REQ_MAGIC);
+	EXPECT_EQ(msg_read->type, PB_CMD_MAGIC);
 	EXPECT_EQ(msg_read->sender, 0);
 	EXPECT_NE(msg_read->msg, nullptr);
-	pb_cmd_res_magic_t * magic = (pb_cmd_res_magic_t *) msg_read->msg;
+	pb_cmd_magic_t * magic = (pb_cmd_magic_t *) msg_read->msg;
 	EXPECT_EQ(magic->_magic_size, sizeof(pb_cmd_magic_req));
 	EXPECT_EQ(0, memcmp(pb_cmd_magic_req, magic->magic, magic->_magic_size));