API: Deprecate OSDP_CARD_FMT_ASCII,

The underlying OSDP response osdp_FMT was underspecified by SIA and later deprecated in v2.2.2. Since supporting this response leads to confusions about to interpret the data, we will stop sending/parsing this response. To maintain, API compatibility, we will keep OSDP_CARD_FMT_ASCII around but internally it gets nop-ed out. Related-to: #206 Signed-off-by: Siddharth Chandrasekaran <[email protected]>
goToMain · Dec 5, 2024 · e1d052c · e1d052c
1 parent c70dd75
commit e1d052c
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 49 deletions.
diff --git a/include/osdp.h b/include/osdp.h
@@ -741,17 +741,12 @@ struct osdp_cmd {
 enum osdp_event_cardread_format_e {
 	OSDP_CARD_FMT_RAW_UNSPECIFIED, /**< Unspecified card format */
 	OSDP_CARD_FMT_RAW_WIEGAND,     /**< Wiegand card format */
-	OSDP_CARD_FMT_ASCII,           /**< ASCII card format */
+	OSDP_CARD_FMT_ASCII,           /**< ASCII card format (deprecated; don't use) */
 	OSDP_CARD_FMT_SENTINEL         /**< Max card format value */
 };
 
 /**
  * @brief OSDP event cardread
- *
- * @note When @a format is set to OSDP_CARD_FMT_RAW_UNSPECIFIED or
- * OSDP_CARD_FMT_RAW_WIEGAND, the length is expressed in bits. OTOH, when it is
- * set to OSDP_CARD_FMT_ASCII, the length is in bytes. The number of bytes to
- * read from the @a data field must be interpreted accordingly.
  */
 struct osdp_event_cardread {
 	/**
@@ -769,7 +764,7 @@ struct osdp_event_cardread {
 	 */
 	int direction;
 	/**
-	 * Length of card data in bytes or bits depending on @a format
+	 * Length of card data in bits
 	 */
 	int length;
 	/**

diff --git a/src/osdp_common.h b/src/osdp_common.h
@@ -229,7 +229,7 @@ union osdp_ephemeral_data {
 #define REPLY_OSTATR	0x4A
 #define REPLY_RSTATR	0x4B
 #define REPLY_RAW	0x50
-#define REPLY_FMT	0x51
+#define REPLY_FMT	0x51 /* deprecated */
 #define REPLY_KEYPAD	0x53
 #define REPLY_COM	0x54
 #define REPLY_BIOREADR	0x57

diff --git a/src/osdp_cp.c b/src/osdp_cp.c
@@ -41,7 +41,6 @@
 #define REPLY_RMAC_I_DATA_LEN          16
 #define REPLY_KEYPAD_DATA_LEN          2   /* variable length command */
 #define REPLY_RAW_DATA_LEN             4   /* variable length command */
-#define REPLY_FMT_DATA_LEN             3   /* variable length command */
 #define REPLY_BUSY_DATA_LEN            0
 #define REPLY_MFGREP_LEN               4   /* variable length command */
 
@@ -591,21 +590,14 @@ static int cp_decode_response(struct osdp_pd *pd, uint8_t *buf, int len)
 		ret = OSDP_CP_ERR_NONE;
 		break;
 	case REPLY_FMT:
-		if (len < REPLY_FMT_DATA_LEN) {
-			break;
-		}
-		event.type = OSDP_EVENT_CARDREAD;
-		event.cardread.reader_no = buf[pos++];
-		event.cardread.direction = buf[pos++];
-		event.cardread.length = buf[pos++];
-		event.cardread.format = OSDP_CARD_FMT_ASCII;
-		if (event.cardread.length != (len - REPLY_FMT_DATA_LEN) ||
-		    event.cardread.length > OSDP_EVENT_CARDREAD_MAX_DATALEN) {
-			break;
-		}
-		memcpy(event.cardread.data, buf + pos, event.cardread.length);
-		memcpy(pd->ephemeral_data, &event, sizeof(event));
-		make_request(pd, CP_REQ_EVENT_SEND);
+		/**
+		 * osdp_FMT was underspecifed by SIA from get-go. It was marked
+		 * for deprecation in v2.2.2. To avoid confusions, we will just
+		 * ignore it here.
+		 *
+		 * See: https://github.com/goToMain/libosdp/issues/206
+		 */
+		LOG_WRN("Ignoring deprecated response osdp_FMT");
 		ret = OSDP_CP_ERR_NONE;
 		break;
 	case REPLY_BUSY:
@@ -1033,7 +1025,6 @@ static bool cp_check_online_response(struct osdp_pd *pd)
 		    pd->reply_id == REPLY_RSTATR ||
 		    pd->reply_id == REPLY_MFGREP ||
 		    pd->reply_id == REPLY_RAW ||
-		    pd->reply_id == REPLY_FMT ||
 		    pd->reply_id == REPLY_KEYPAD) {
 			return true;
 		}

diff --git a/src/osdp_pd.c b/src/osdp_pd.c
@@ -44,7 +44,6 @@
 #define REPLY_RMAC_I_LEN               17
 #define REPLY_KEYPAD_LEN               2
 #define REPLY_RAW_LEN                  4
-#define REPLY_FMT_LEN                  3
 #define REPLY_MFGREP_LEN               4 /* variable length command */
 
 enum osdp_pd_error_e {
@@ -148,7 +147,14 @@ static int pd_translate_event(struct osdp_pd *pd, struct osdp_event *event)
 		    event->cardread.format == OSDP_CARD_FMT_RAW_WIEGAND) {
 			reply_code = REPLY_RAW;
 		} else if (event->cardread.format == OSDP_CARD_FMT_ASCII) {
-			reply_code = REPLY_FMT;
+			/**
+			 * osdp_FMT was underspecifed by SIA from get-go. It
+			 * was marked for deprecation in v2.2.2.
+			 *
+			 * See: https://github.com/goToMain/libosdp/issues/206
+			 */
+			LOG_WRN("Event CardRead::format::OSDP_CARD_FMT_ASCII"
+				" is deprecated. Ignoring");
 		} else {
 			LOG_ERR("Event: cardread; Error: unknown format");
 			break;
@@ -812,17 +818,6 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len)
 		ret = OSDP_PD_ERR_NONE;
 		break;
 	}
-	case REPLY_FMT:
-		event = (struct osdp_event *)pd->ephemeral_data;
-		assert_buf_len(REPLY_FMT_LEN + event->cardread.length, max_len);
-		buf[len++] = pd->reply_id;
-		buf[len++] = (uint8_t)event->cardread.reader_no;
-		buf[len++] = (uint8_t)event->cardread.direction;
-		buf[len++] = (uint8_t)event->cardread.length;
-		memcpy(buf + len, event->cardread.data, event->cardread.length);
-		len += event->cardread.length;
-		ret = OSDP_PD_ERR_NONE;
-		break;
 	case REPLY_COM:
 		assert_buf_len(REPLY_COM_LEN, max_len);
 		/**

diff --git a/tests/pytest/test_events.py b/tests/pytest/test_events.py
@@ -77,17 +77,6 @@ def test_event_mfg_reply():
     secure_pd.notify_event(event)
     check_event(event)
 
-def test_event_cardread_ascii():
-    event = {
-        'event': Event.CardRead,
-        'reader_no': 1,
-        'direction': 1,
-        'format': CardFormat.ASCII,
-        'data': bytes([9,1,9,2,6,3,1,7,7,0]),
-    }
-    secure_pd.notify_event(event)
-    check_event(event)
-
 def test_event_cardread_wiegand():
     event = {
         'event': Event.CardRead,