From 7e92957ef9c5d30878aded0c52e7aee99b8b9a9e Mon Sep 17 00:00:00 2001 From: George Norton Date: Thu, 1 Jun 2023 20:43:10 +0100 Subject: [PATCH] Continuted to implement communication protocol. Can now validate Filter Configuration Structures --- firmware/code/configuration_manager.c | 141 ++++++++++++++++++++++---- firmware/code/configuration_manager.h | 14 +++ firmware/code/configuration_types.h | 14 +++ firmware/code/os_descriptors.h | 47 +++++++-- firmware/code/run.c | 2 - 5 files changed, 186 insertions(+), 32 deletions(-) diff --git a/firmware/code/configuration_manager.c b/firmware/code/configuration_manager.c index 5eb22c8..1ad7d94 100644 --- a/firmware/code/configuration_manager.c +++ b/firmware/code/configuration_manager.c @@ -1,3 +1,17 @@ +/** + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ #include #include #include @@ -8,6 +22,27 @@ #include "configuration_manager.h" #include "configuration_types.h" +// TODO: Duplicated from os_descriptors.h +#define U16_HIGH(_u16) ((uint8_t) (((_u16) >> 8) & 0x00ff)) +#define U16_LOW(_u16) ((uint8_t) ((_u16) & 0x00ff)) + +#define U16_TO_U8S_LE(_u16) U16_LOW(_u16), U16_HIGH(_u16) + +/** + * We have multiple copies of the device configuration. This is the factory + * default configuration, it is static data in the firmware. + * We also potentially have a user configuration stored at the end of flash + * memory. And an in RAM working configuration. + * + * The idea is that when the device boots, it tries to use the user config + * from the end of flash. If that is not present, or is invalid, we use this + * default config instead. + * + * If the user sends an updated configuration over the USB port, it is stored + * in RAM as a working configuration, and is used (until we lose power). If + * the user issues a save command the working configuration is written to flash + * and becomes the new user configuration. + */ static const default_configuration default_config = { .filters = { .filter = { FILTER_CONFIGURATION, sizeof(default_config.filters) }, @@ -19,12 +54,17 @@ static const default_configuration default_config = { } }; -static uint8_t config_buffer[2][512]; -static uint8_t active_index = 0; +/** + * TODO: For now, assume we always get a complete configuration but maybe we + * should handle merging configurations where, for example, only a new + * filter_configuration_tlv was received. + */ +static uint8_t working_configuration[256]; +static uint8_t result_buffer[256] = { U16_TO_U8S_LE(NOK), U16_TO_U8S_LE(4) }; + static bool config_dirty = false; static uint16_t write_offset = 0; static uint16_t read_offset = 0; -static uint16_t config_length = 0; bool validate_filter_configuration(filter_configuration_tlv *filters) { @@ -55,6 +95,7 @@ bool validate_filter_configuration(filter_configuration_tlv *filters) return false; } filter2 *args = (filter2 *)ptr; + printf("Args: F0: %0.2f, Q: %0.2f\n", args->f0, args->Q); ptr += sizeof(filter2); break; } @@ -68,7 +109,7 @@ bool validate_filter_configuration(filter_configuration_tlv *filters) return false; } filter3 *args = (filter3 *)ptr; - printf("Args: %0.2f %0.2f, %0.2f\n", args->f0, args->dBgain, args->Q); + printf("Args: F0: %0.2f, dbGain: %0.2f, Q: %0.2f\n", args->f0, args->dBgain, args->Q); ptr += sizeof(filter3); break; } @@ -86,13 +127,32 @@ bool validate_filter_configuration(filter_configuration_tlv *filters) return true; } +bool validate_configuration(tlv_header *config) +{ + if (config->type != SET_CONFIGURATION) { + printf("Unexpcected Config type: %d\n", config->type); + return false; + } + uint8_t *ptr = (uint8_t *)config->value; + const uint8_t *end = (uint8_t *)config + config->length; + while (ptr < end) { + tlv_header* tlv = (tlv_header*) ptr; + printf("Found TLV type: %d\n", tlv->type); + if (tlv->type == FILTER_CONFIGURATION) + { + if (!validate_filter_configuration((filter_configuration_tlv*) tlv)) { + return false; + } + } + + ptr += tlv->length; + } +} + void load_config() { - // Copy data from flash - uint8_t index = active_index ? 0 : 1; - // Load data - active_index = index; - config_dirty = true; + // Try to load data from flash + // If that is no good, use the default config } void save_config() @@ -100,24 +160,65 @@ void save_config() // Write data to flash } -void config_in_packet(struct usb_endpoint *ep) { - assert(ep->current_transfer); - struct usb_buffer *buffer = usb_current_in_packet_buffer(ep); - assert(buffer->data_max >= 3); - buffer->data_len = 0; +// This callback is called when the client sends a message to the device. +// We implement a simple messaging protocol. The client sends us a message that +// we consume here. All messages are constructed of TLV's (Type Length Value). +// In some cases the Value may be a set of TLV's. However, each message has an +// owning TLV, and its length determines the length of the transfer. +// Once we have consumed the whole message, we validate it and populate the result +// buffer with a TLV which we expect the client to read next. +void config_out_packet(struct usb_endpoint *ep) { + struct usb_buffer *buffer = usb_current_out_packet_buffer(ep); + printf("config_out_packet %d\n", buffer->data_len); - printf("Config In Packet: Buffer Len: %d, Max: %d\n", buffer->data_len, buffer->data_max); - memcpy(buffer->data, "Config In Packet", 17); - buffer->data_len = 64; + memcpy(&working_configuration[write_offset], buffer->data, buffer->data_len); + write_offset += buffer->data_len; + + const uint16_t transfer_length = ((tlv_header*) working_configuration)->length; + printf("config_length %d %d\n", transfer_length, write_offset); + if (transfer_length >= write_offset) { + // Command complete, fill the result buffer + tlv_header* result = ((tlv_header*) result_buffer); + write_offset = 0; + + if (validate_configuration((tlv_header*) working_configuration)) { + result->type = OK; + result->length = 4; + } + else { + result->type = NOK; + result->length = 4; + } + } usb_grow_transfer(ep->current_transfer, 1); usb_packet_done(ep); } -void config_out_packet(struct usb_endpoint *ep) { - struct usb_buffer *usb_buffer = usb_current_out_packet_buffer(ep); +// This callback is called when the client attempts to read data from the device. +// The client should have previously written a command which will have populated the +// result_buffer. The client should attempt to read 4 bytes (the Type and Length) +// then attempt to read the rest of the data once the length is known. +void config_in_packet(struct usb_endpoint *ep) { + assert(ep->current_transfer); + struct usb_buffer *buffer = usb_current_in_packet_buffer(ep); + printf("config_in_packet %d\n", buffer->data_len); + assert(buffer->data_max >= 3); - printf("Config Out Packet >>%s<< %d %d %d %d\n", usb_buffer->data, usb_buffer->data_len, ep->current_transfer->completed, ep->current_transfer->remaining_packets_to_handle, ep->current_transfer->remaining_packets_to_submit); + const uint16_t transfer_length = ((tlv_header*) result_buffer)->length; + const uint16_t packet_length = MIN(buffer->data_max, transfer_length - read_offset); + memcpy(buffer->data, &result_buffer[read_offset], packet_length); + buffer->data_len = packet_length; + + if (transfer_length >= read_offset) { + // Done + read_offset = 0; + + // If the client reads again, return an error + tlv_header* result = ((tlv_header*) result_buffer); + result->type = NOK; + result->length = 4; + } usb_grow_transfer(ep->current_transfer, 1); usb_packet_done(ep); diff --git a/firmware/code/configuration_manager.h b/firmware/code/configuration_manager.h index 1834af4..a8e1a8c 100644 --- a/firmware/code/configuration_manager.h +++ b/firmware/code/configuration_manager.h @@ -1,3 +1,17 @@ +/** + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ #ifndef CONFIGURATION_MANAGER_H #define CONFIGURATION_MANAGER_H struct usb_endpoint; diff --git a/firmware/code/configuration_types.h b/firmware/code/configuration_types.h index 89a562f..afaa357 100644 --- a/firmware/code/configuration_types.h +++ b/firmware/code/configuration_types.h @@ -1,3 +1,17 @@ +/** + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ #ifndef __CONFIGURATION_TYPES_H__ #define __CONFIGURATION_TYPES_H__ #include diff --git a/firmware/code/os_descriptors.h b/firmware/code/os_descriptors.h index dcb3d76..248eb7c 100644 --- a/firmware/code/os_descriptors.h +++ b/firmware/code/os_descriptors.h @@ -1,19 +1,44 @@ +/** + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + #ifndef __OS_DESCRIPTORS__ #define __OS_DESCRIPTORS__ +/** + * This stuff is all required to get a WinUSB driver on the control interface on Windows. + * Without this it is impossible to communicate with the device using libusb. + * + * During device enumeration the OS will request the platform capability binary object store + * (BOS) descriptor. This descriptor contains a magic UUID which signals the device supports + * the Microsoft OS 2.0 capability descriptor. It also signals the "Vendor code" and the size + * of the Microsoft OS 2.0 capability descriptor. + * Next Windows will request the Microsoft OS 2.0 capability descriptor by issuing a device + * vendor setup request, with bRequest equal to the "Vendor code" from the BOS descriptor and + * wIndex == 7. We handle this in ad_setup_request_handler() in run.c. + */ #include -#define TU_U16_HIGH(_u16) ((uint8_t) (((_u16) >> 8) & 0x00ff)) -#define TU_U16_LOW(_u16) ((uint8_t) ((_u16) & 0x00ff)) -#define U16_TO_U8S_BE(_u16) TU_U16_HIGH(_u16), TU_U16_LOW(_u16) -#define U16_TO_U8S_LE(_u16) TU_U16_LOW(_u16), TU_U16_HIGH(_u16) -#define TU_U32_BYTE3(_u32) ((uint8_t) ((((uint32_t) _u32) >> 24) & 0x000000ff)) // MSB -#define TU_U32_BYTE2(_u32) ((uint8_t) ((((uint32_t) _u32) >> 16) & 0x000000ff)) -#define TU_U32_BYTE1(_u32) ((uint8_t) ((((uint32_t) _u32) >> 8) & 0x000000ff)) -#define TU_U32_BYTE0(_u32) ((uint8_t) (((uint32_t) _u32) & 0x000000ff)) // LSB +#define U16_HIGH(_u16) ((uint8_t) (((_u16) >> 8) & 0x00ff)) +#define U16_LOW(_u16) ((uint8_t) ((_u16) & 0x00ff)) -#define U32_TO_U8S_BE(_u32) TU_U32_BYTE3(_u32), TU_U32_BYTE2(_u32), TU_U32_BYTE1(_u32), TU_U32_BYTE0(_u32) -#define U32_TO_U8S_LE(_u32) TU_U32_BYTE0(_u32), TU_U32_BYTE1(_u32), TU_U32_BYTE2(_u32), TU_U32_BYTE3(_u32) +#define U32_BYTE3(_u32) ((uint8_t) ((((uint32_t) _u32) >> 24) & 0x000000ff)) // MSB +#define U32_BYTE2(_u32) ((uint8_t) ((((uint32_t) _u32) >> 16) & 0x000000ff)) +#define U32_BYTE1(_u32) ((uint8_t) ((((uint32_t) _u32) >> 8) & 0x000000ff)) +#define U32_BYTE0(_u32) ((uint8_t) (((uint32_t) _u32) & 0x000000ff)) // LSB +#define U16_TO_U8S_LE(_u16) U16_LOW(_u16), U16_HIGH(_u16) +#define U32_TO_U8S_LE(_u32) U32_BYTE0(_u32), U32_BYTE1(_u32), U32_BYTE2(_u32), U32_BYTE3(_u32) #define MS_OS_20_DESC_LEN 0xB2 typedef enum { @@ -28,6 +53,8 @@ typedef enum { MS_OS_20_FEATURE_VENDOR_REVISION = 0x08 } microsoft_os_20_type_t; +// Warning: The USB stack expects these descriptors to be a multiple of 64 bytes. Also the offset +// computation breaks down if the size is not a power of two. static __aligned(4) uint8_t ms_platform_capability_bos_descriptor[PICO_USBDEV_MAX_DESCRIPTOR_SIZE] = { // BOS Descriptor // length, descriptor type, total length, number of device caps diff --git a/firmware/code/run.c b/firmware/code/run.c index ae901b4..885ca6e 100644 --- a/firmware/code/run.c +++ b/firmware/code/run.c @@ -194,8 +194,6 @@ void setup() { pico_get_unique_board_id_string(spi_serial_number, 17); descriptor_strings[2] = spi_serial_number; - printf("Serial Number: %s (%p)\n", descriptor_strings[2], descriptor_strings[2]); - userbuf = malloc(sizeof(uint8_t) * RINGBUF_LEN_IN_BYTES); // Configure DAC PWM