Merge pull request #130 from rust-embedded-community/feature/language-refactor

Refactoring language management for simplicity

Author: ryan-summers

CHANGELOG.md

@@ -21,7 +21,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Breaking
 * Acess numeric form of `EndpointType` variants now require a `.to_bm_attributes()`. ([#60](https://github.com/rust-embedded-community/usb-device/pull/60))
 * `DescriptorWriter::iad()` now requires a `Option<StringIndex>` to optionally specify a string for describing the function ([#121](https://github.com/rust-embedded-community/usb-device/pull/121))
-* `.manufacturer()`, `.product()` and `.serial_number()` of `UsbDeviceBuilder` now require `&[&str]` to specify strings match with each LANGIDs supported by device. ([#122](https://github.com/rust-embedded-community/usb-device/pull/122))
+* `.manufacturer()`, `.product()` and `.serial_number()` of `UsbDeviceBuilder` are now replaced with the `strings()` function that accepts a `StringDescriptor` list to allow multilanguage support ([#122](https://github.com/rust-embedded-community/usb-device/pull/122))
+* Various methods of the `UsbDeviceBuilder` now return `Result<>` types instead of internally panicking.
 
 ### Changed
 * `EndpointType` enum now has fields for isochronous synchronization and usage ([#60](https://github.com/rust-embedded-community/usb-device/pull/60)).

Cargo.toml

@@ -13,6 +13,7 @@ repository = "https://github.com/mvirkkunen/usb-device"
 defmt = { version = "0.3", optional = true }
 portable-atomic = { version = "1.2.0", default-features = false }
 num_enum = { version = "0.6.1", default-features = false }
+heapless = "0.7"
 
 [dev-dependencies]
 rusb = "0.9.1"

src/descriptor.rs

@@ -116,11 +116,29 @@ impl DescriptorWriter<'_> {
                 config.product_id as u8,
                 (config.product_id >> 8) as u8, // idProduct
                 config.device_release as u8,
-                (config.device_release >> 8) as u8,    // bcdDevice
-                config.manufacturer.map_or(0, |_| 1),  // iManufacturer
-                config.product.map_or(0, |_| 2),       // iProduct
-                config.serial_number.map_or(0, |_| 3), // iSerialNumber
-                1,                                     // bNumConfigurations
+                (config.device_release >> 8) as u8, // bcdDevice
+                config.string_descriptors.first().map_or(0, |lang| {
+                    if lang.manufacturer.is_some() {
+                        1
+                    } else {
+                        0
+                    }
+                }),
+                config.string_descriptors.first().map_or(0, |lang| {
+                    if lang.product.is_some() {
+                        2
+                    } else {
+                        0
+                    }
+                }),
+                config.string_descriptors.first().map_or(0, |lang| {
+                    if lang.serial.is_some() {
+                        3
+                    } else {
+                        0
+                    }
+                }),
+                1, // bNumConfigurations
             ],
         )
     }

src/descriptor/lang_id.rs

@@ -15,7 +15,7 @@ impl From<LangID> for u16 {
 }
 
 #[allow(missing_docs)]
-#[derive(Clone, Copy, PartialEq, TryFromPrimitive)]
+#[derive(Clone, Copy, PartialEq, Debug, TryFromPrimitive)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 #[repr(u16)]
 pub enum LangID {

src/device.rs

@@ -3,7 +3,7 @@ use crate::class::{ControlIn, ControlOut, UsbClass};
 use crate::control;
 use crate::control_pipe::ControlPipe;
 use crate::descriptor::{descriptor_type, lang_id::LangID, BosWriter, DescriptorWriter};
-pub use crate::device_builder::{UsbDeviceBuilder, UsbVidPid};
+pub use crate::device_builder::{StringDescriptors, UsbDeviceBuilder, UsbVidPid};
 use crate::endpoint::{EndpointAddress, EndpointType};
 use crate::{Result, UsbDirection};
 use core::convert::TryFrom;
@@ -64,10 +64,7 @@ pub(crate) struct Config<'a> {
     pub product_id: u16,
     pub usb_rev: UsbRev,
     pub device_release: u16,
-    pub extra_lang_ids: Option<&'a [LangID]>,
-    pub manufacturer: Option<&'a [&'a str]>,
-    pub product: Option<&'a [&'a str]>,
-    pub serial_number: Option<&'a [&'a str]>,
+    pub string_descriptors: heapless::Vec<StringDescriptors<'a>, 16>,
     pub self_powered: bool,
     pub supports_remote_wakeup: bool,
     pub composite_with_iads: bool,
@@ -548,97 +545,66 @@ impl<B: UsbBus> UsbDevice<'_, B> {
             descriptor_type::STRING => match index {
                 // first STRING Request
                 0 => {
-                    if let Some(extra_lang_ids) = config.extra_lang_ids {
-                        let mut lang_id_bytes = [0u8; 32];
-
-                        lang_id_bytes
-                            .chunks_exact_mut(2)
-                            .zip([LangID::EN_US].iter().chain(extra_lang_ids.iter()))
-                            .for_each(|(buffer, lang_id)| {
-                                buffer.copy_from_slice(&u16::from(lang_id).to_le_bytes());
-                            });
-
-                        accept_writer(xfer, |w| {
-                            w.write(
-                                descriptor_type::STRING,
-                                &lang_id_bytes[0..(1 + extra_lang_ids.len()) * 2],
-                            )
-                        })
-                    } else {
-                        accept_writer(xfer, |w| {
-                            w.write(
-                                descriptor_type::STRING,
-                                &u16::from(LangID::EN_US).to_le_bytes(),
-                            )
-                        })
+                    let mut lang_id_bytes = [0u8; 32];
+                    for (lang, buf) in config
+                        .string_descriptors
+                        .iter()
+                        .zip(lang_id_bytes.chunks_exact_mut(2))
+                    {
+                        buf.copy_from_slice(&u16::from(lang.id).to_le_bytes());
                     }
+                    accept_writer(xfer, |w| {
+                        w.write(
+                            descriptor_type::STRING,
+                            &lang_id_bytes[..config.string_descriptors.len() * 2],
+                        )
+                    })
                 }
 
                 // rest STRING Requests
                 _ => {
-                    let s = match LangID::try_from(req.index) {
+                    let lang_id = match LangID::try_from(req.index) {
                         Err(_err) => {
                             #[cfg(feature = "defmt")]
                             defmt::warn!(
                                 "Receive unknown LANGID {:#06X}, reject the request",
                                 _err.number
                             );
-                            None
+                            xfer.reject().ok();
+                            return;
                         }
 
-                        Ok(req_lang_id) => {
-                            if index <= 3 {
-                                // for Manufacture, Product and Serial
-
-                                // construct the list of lang_ids full supported by device
-                                let mut lang_id_list: [Option<LangID>; 16] = [None; 16];
-                                match config.extra_lang_ids {
-                                    None => lang_id_list[0] = Some(LangID::EN_US),
-                                    Some(extra_lang_ids) => {
-                                        lang_id_list
-                                            .iter_mut()
-                                            .zip(
-                                                [LangID::EN_US].iter().chain(extra_lang_ids.iter()),
-                                            )
-                                            .for_each(|(item, lang_id)| *item = Some(*lang_id));
-                                    }
-                                };
-
-                                let position =
-                                    lang_id_list.iter().fuse().position(|list_lang_id| {
-                                        matches!(*list_lang_id, Some(list_lang_id) if req_lang_id == list_lang_id)
-                                    });
-                                #[cfg(feature = "defmt")]
-                                if position.is_none() {
-                                    // Since we construct the list of full supported lang_ids previously,
-                                    // we can safely reject requests which ask for other lang_id.
-                                    defmt::warn!(
-                                        "Receive unknown LANGID {:#06X}, reject the request",
-                                        req_lang_id
-                                    );
-                                }
-                                position.and_then(|lang_id_list_index| {
-                                    match index {
-                                        1 => config.manufacturer,
-                                        2 => config.product,
-                                        3 => config.serial_number,
-                                        _ => unreachable!(),
-                                    }
-                                    .map(|str_list| str_list[lang_id_list_index])
-                                })
-                            } else {
-                                // for other custom STRINGs
-
-                                let index = StringIndex::new(index);
-                                classes
-                                    .iter()
-                                    .find_map(|cls| cls.get_string(index, req_lang_id))
+                        Ok(req_lang_id) => req_lang_id,
+                    };
+                    let string = match index {
+                        // Manufacturer, product, and serial are handled directly here.
+                        1..=3 => {
+                            let Some(lang) = config
+                                .string_descriptors
+                                .iter()
+                                .find(|lang| lang.id == lang_id)
+                            else {
+                                xfer.reject().ok();
+                                return;
+                            };
+
+                            match index {
+                                1 => lang.manufacturer,
+                                2 => lang.product,
+                                3 => lang.serial,
+                                _ => unreachable!(),
                             }
                         }
+                        _ => {
+                            let index = StringIndex::new(index);
+                            classes
+                                .iter()
+                                .find_map(|cls| cls.get_string(index, lang_id))
+                        }
                     };
 
-                    if let Some(s) = s {
-                        accept_writer(xfer, |w| w.string(s));
+                    if let Some(string_descriptor) = string {
+                        accept_writer(xfer, |w| w.string(string_descriptor));
                     } else {
                         let _ = xfer.reject();
                     }