Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Clarify protocol. Disruptive changes: see below
This huge commit tries to enhance several things related to the fw/cli protocol. Generally, the idea is to be verbose, explicit and reuse conventions, in order to enhance code maintainability and understandability for the other contributors. docs/protocol.md got heavily updated Guidelines, also written in docs/protocol.md "New data payloads: guidelines for developers": - Now protocol data exchanged over USB or BLE are defined in netdata.h as packed structs and values are stored in Network byte order (=Big Endian) - Command-specific payloads are defined in their respective cmd_processor handler in app_cmd.c and chameleon_cmd.py - Define C `struct` for cmd/resp data greater than a single byte, use and abuse of `struct.pack`/`struct.unpack` in Python. So one can understand the payload format at a simple glimpse. - If single byte of data to return, still use a 1-byte `data`, not `status`. - Use unambiguous types such as `uint16_t`, not `int` or `enum`. Cast explicitly `int` and `enum` to `uint_t` of proper size - Use Network byte order for 16b and 32b integers - Macros `U16NTOHS`, `U32NTOHL` must be used on reception of a command payload. - Macros `U16HTONS`, `U32HTONL` must be used on creation of a response payload. - In Python, use the modifier `!` with all `struct.pack`/`struct.unpack` - Concentrate payload parsing in the handlers, avoid further parsing in their callers. This is true for the firmware and the client. - In cmd_processor handlers: don't reuse input `length`/`data` parameters for creating the response content - Avoid hardcoding offsets, use `sizeof()`, `offsetof(struct, field)` in C and `struct.calcsize()` in Python - Use the exact same command and fields names in firmware and in client, use function names matching the command names for their handlers unless there is a very good reason not to do so. This helps grepping around. Names must start with a letter, not a number, because some languages require it (e.g. `14a_scan` not possible in Python) - Respect commands order in `m_data_cmd_map`, `data_cmd.h` and `chameleon_cmd.py` definitions - Even if a command is not yet implemented in firmware or in client but a command number is allocated, add it to `data_cmd.h` and `chameleon_cmd.py` with some `FIXME: to be implemented` comment - Validate data before using it, both when receiving command data in the firmware and when receiving response data in the client. - Validate response status in client. Disruptive changes: - GET_DEVICE_CAPABILITIES: list of cmds in data are now really Big Endian Note: the initial attempt to use macros PP_HTONS were actually considering wrongly that the platform was Big Endian (BYTE_ORDER was actually undefined) while it is actually Little Endian. - GET_APP_VERSION: response is now a tuple of bytes: major|minor (previously it was in reversed order as a single uint16_t in Little Endian) - SET_SLOT_TAG_TYPE: tag_type now on 2 bytes, to prepare remapping of its enum - SET_SLOT_DATA_DEFAULT: tag_type now on 2 bytes, to prepare remapping of its enum - GET_SLOT_INFO: tag_type now on 2 bytes, to prepare remapping of its enum - GET_DEVICE_CHIP_ID: now returns its 64b ID following Network byte order (previously, bytes were in the reverse order) - GET_DEVICE_ADDRESS: now returns its 56b address following Network byte order (previously, bytes were in the reverse order). CLI does not reverse the response anymore so it displays the same value as before. - GET_GIT_VERSION response status is now STATUS_DEVICE_SUCCESS - GET_DEVICE_MODEL response status is now STATUS_DEVICE_SUCCESS - MF1_READ_EMU_BLOCK_DATA response status is now STATUS_DEVICE_SUCCESS - GET_DEVICE_CAPABILITIES response status is now STATUS_DEVICE_SUCCESS - HF14A_SCAN: entirely new response format, room for ATS and multiple tags - MF1_DETECT_SUPPORT response status is now HF_TAG_OK and support is indicated as bool in 1 byte of data - MF1_DETECT_PRNG response status is now HF_TAG_OK and prng_type is returned in 1 byte of data with a new enum mf1_prng_type_t == MifareClassicPrngType - MF1_DETECT_DARKSIDE response status is now HF_TAG_OK and darkside_status is returned in 1 byte of data with a new enum mf1_darkside_status_t == MifareClassicDarksideStatus - MF1_DARKSIDE_ACQUIRE response status is now HF_TAG_OK and darkside_status is returned in 1 byte of data. If OK, followed by 24 bytes as previously - not breaking but many commands have been renamed for consistency. you are invited to adapt your client for easier maintenance Other changes: - FW/CLI: changes to match the aforementioned guidelines - CLI: does not instanciate ChameleonCMD on every single command - CLI: query capabilities on connect, not on every single command if device does not support the get_device_capabilities command - CLI: hw raw: detail status messages - CLI: merge #143 manually - CLI: HF14AInfo logic moved inside HF14AScan - FW: replace dynamic cmd_map_init() by static cmd_map initialization - probably many little ones I forgot about... TODO: - remap `tag_specific_type_t` enum to allow future tags (e.g. LF tags) without reshuffling enum and affecting users stored cards - continue cmd list review, cf protocol.md - document cmd in protocol.md - apply recommendations given in "New data payloads: guidelines for developers" - use @expect_response_ng, temporary name as it should replace @expect_response once done everywhere - rewrite cmd pack if needed - rewrite cmd unpack if needed - rewrite resp pack if needed, replace num_to_bytes - rewrite resp unpack if needed, limit data parsing to cmd handlers, revise chameleon_cstruct - TEST!
- Loading branch information
