✨ (minor) Add control_endpoint::has_setup() and explain interface::handle_request error handling (libhal#200)

* ✨ usb: add has_setup() and explain handle_request error handling

- Add `has_setup()` method to control endpoint with optional<bool> return
  for implementations that don't support setup packet detection
- Add `driver_has_setup()` virtual with default nullopt implementation
- Move `setup_packet` offset constants (`value_offset`, `index_offset`,
  `length_offset`) to top of struct and use them in all byte-access methods
- Rename `p_ep_req` parameters to `p_endpoint` in `usb::interface` APIs
- Expand `handle_request()` docs to explain setup-abort exception protocol
- Document `operation_not_permitted` throws in `read()` and `write()`
- Drop redundant `enum` keyword in `static_cast<request_type>(...)`
- Drop `hal::` qualifier on `callback<>` in `on_receive()`
- Add `has_setup` tests to `usb.test.cpp` and include `<optional>`

* ✏️ Give clarity to docs & add missing test

* ✏️ Elaborate on the throws conditions for endpoint_io

Author: kammce

v4/include/libhal/usb.hpp

@@ -255,18 +255,47 @@ class control_endpoint : public endpoint
    * @param p_callback The callback function to be called when a USB request
    * command is received on the control endpoint.
    */
-  void on_receive(hal::callback<void(on_receive_tag)> const& p_callback)
+  void on_receive(callback<void(on_receive_tag)> const& p_callback)
   {
     driver_on_receive(p_callback);
   }
 
+  /**
+   * @brief Returns whether the endpoint's receive buffer contains a setup
+   * packet.
+   *
+   * Returns `true` until all 8 bytes of the setup packet have been extracted
+   * via `read()`. Returns `false` once the setup packet has been fully consumed
+   * or if the last received packet was not a setup packet.
+   *
+   * Returns `std::nullopt` if the implementation does not support setup packet
+   * detection. Enumerators that require this information should accept a
+   * callable (as a template parameter or type-erased input parameter) to work
+   * around legacy implementations.
+   *
+   * @note Safe to call from ISR context. Implementations must not throw,
+   * call throwing functions, or perform I/O within this method.
+   *
+   * @return `std::nullopt` if setup packet detection is unsupported.
+   * @return `true` if the receive buffer contains an unconsumed setup packet.
+   * @return `false` otherwise.
+   */
+  [[nodiscard]] std::optional<bool> has_setup() const noexcept
+  {
+    return driver_has_setup();
+  }
+
 private:
   virtual void driver_connect(bool p_should_connect) = 0;
   virtual void driver_set_address(u8 p_address) = 0;
   virtual void driver_write(scatter_span<byte const> p_data) = 0;
   virtual usize driver_read(scatter_span<byte> p_buffer) = 0;
   virtual void driver_on_receive(
     callback<void(on_receive_tag)> const& p_callback) = 0;
+  [[nodiscard]] virtual std::optional<bool> driver_has_setup() const noexcept
+  {
+    return {};
+  }
 };
 
 /**
@@ -454,6 +483,10 @@ concept in_endpoint_type =
  */
 struct setup_packet
 {
+  constexpr static usize value_offset = 2;
+  constexpr static usize index_offset = 4;
+  constexpr static usize length_offset = 6;
+
   /**
    * @brief Request type classification for setup packets
    *
@@ -578,7 +611,7 @@ struct setup_packet
       return request_type::invalid;
     }
 
-    return static_cast<enum request_type>(t);
+    return static_cast<request_type>(t);
   }
 
   /**
@@ -629,47 +662,50 @@ struct setup_packet
    */
   [[nodiscard]] constexpr u16 value() const
   {
-    return from_le_bytes(raw_request_bytes[2], raw_request_bytes[3]);
+    return from_le_bytes(raw_request_bytes[value_offset],
+                         raw_request_bytes[value_offset + 1]);
   }
 
   /**
    * @brief Get wValue field as raw LE bytes for USB bus transmission
    */
   [[nodiscard]] constexpr std::span<hal::byte const> value_bytes() const
   {
-    return std::span(raw_request_bytes).subspan(2, 2);
+    return std::span(raw_request_bytes).subspan(value_offset, sizeof(u16));
   }
 
   /**
    * @brief Get wIndex field in native endianness for program use
    */
   [[nodiscard]] constexpr u16 index() const
   {
-    return from_le_bytes(raw_request_bytes[4], raw_request_bytes[5]);
+    return from_le_bytes(raw_request_bytes[index_offset],
+                         raw_request_bytes[index_offset + 1]);
   }
 
   /**
    * @brief Get wIndex field as raw LE bytes for USB bus transmission
    */
   [[nodiscard]] constexpr std::span<hal::byte const> index_bytes() const
   {
-    return std::span(raw_request_bytes).subspan(4, 2);
+    return std::span(raw_request_bytes).subspan(index_offset, sizeof(u16));
   }
 
   /**
    * @brief Get wLength field in native endianness for program use
    */
   [[nodiscard]] constexpr u16 length() const
   {
-    return from_le_bytes(raw_request_bytes[6], raw_request_bytes[7]);
+    return from_le_bytes(raw_request_bytes[length_offset],
+                         raw_request_bytes[length_offset + 1]);
   }
 
   /**
    * @brief Get wLength field as raw LE bytes for USB bus transmission
    */
   [[nodiscard]] constexpr std::span<hal::byte const> length_bytes() const
   {
-    return std::span(raw_request_bytes).subspan(6, 2);
+    return std::span(raw_request_bytes).subspan(length_offset, sizeof(u16));
   }
 
   /**
@@ -751,10 +787,6 @@ struct setup_packet
   }
 
   std::array<byte, 8> raw_request_bytes;
-
-  constexpr static usize value_offset = 2;
-  constexpr static usize index_offset = 4;
-  constexpr static usize length_offset = 6;
 };
 
 /**
@@ -809,7 +841,6 @@ enum class standard_request_types : hal::byte
 
   return static_cast<standard_request_types>(p_packet.request());
 }
-
 /**
  * @brief Endpoint I/O interface for USB request handling
  *
@@ -820,8 +851,17 @@ enum class standard_request_types : hal::byte
  * phase of control transfers.
  *
  * The direction of data transfer depends on the setup packet:
+ *
  * - Device-to-host (IN): Use `write()` to send response data to the host
  * - Host-to-device (OUT): Use `read()` to receive data sent by the host
+ *
+ * @throws hal::operation_not_permitted - If a new setup packet is received
+ * before the current control transfer completes, this exception is thrown to
+ * interrupt the data phase. Only the enumerator should handle this exception;
+ * it must catch it and evaluate the new setup packet. If the underlying
+ * control endpoint implementation does not support setup_packet detection,
+ * this should not be thrown and the enumerator will have to make assumptions
+ * about which packets are setup packets and which are not.
  */
 class endpoint_io
 {
@@ -836,6 +876,7 @@ class endpoint_io
    * endpoint
    * @return usize - the number of bytes read into the provided buffers. Value
    * is 0 if there is no data available within the endpoint.
+   * @throws hal::operation_not_permitted - See class documentation.
    */
   usize read(scatter_span<byte> p_buffer)
   {
@@ -851,6 +892,7 @@ class endpoint_io
    * @param p_buffer - scatter span of const byte buffers containing data to
    * write to the endpoint
    * @return usize - the number of bytes written from the provided buffers
+   * @throws hal::operation_not_permitted - See class documentation.
    */
   usize write(scatter_span<byte const> p_buffer)
   {
@@ -934,13 +976,13 @@ class interface
    * @param p_start - the starting values for interface numbers and string
    * indexes. The `string` field should be cached by the interface in order to
    * allow `write_string_descriptor` to work correctly.
-   * @param p_ep_req - endpoint I/O interface used to write descriptor data to
+   * @param p_endpoint - endpoint I/O interface used to write descriptor data to
    * the host.
    */
   [[nodiscard]] descriptor_count write_descriptors(descriptor_start p_start,
-                                                   endpoint_io& p_ep_req)
+                                                   endpoint_io& p_endpoint)
   {
-    return driver_write_descriptors(p_start, p_ep_req);
+    return driver_write_descriptors(p_start, p_endpoint);
   }
 
   /**
@@ -955,56 +997,72 @@ class interface
    * and descriptor type fields.
    *
    * @param p_index - Which string index's descriptor should be written.
-   * @param p_ep_req - endpoint I/O interface used to write the string
+   * @param p_endpoint - endpoint I/O interface used to write the string
    * descriptor to the host.
    *
    * @returns true - if the string was located and written via the endpoint I/O.
    * @returns false - if the string requested does not belong to this interface.
    */
-  [[nodiscard]] bool write_string_descriptor(u8 p_index, endpoint_io& p_ep_req)
+  [[nodiscard]] bool write_string_descriptor(u8 p_index,
+                                             endpoint_io& p_endpoint)
   {
-    return driver_write_string_descriptor(p_index, p_ep_req);
+    return driver_write_string_descriptor(p_index, p_endpoint);
   }
-
   /**
    * @brief Handle USB requests directed to this interface or its endpoints
    *
-   * This method is called when the USB device receives a setup packet that
-   * targets this interface (interface-specific requests) or one of its
+   * This method is called when the USB device receives a setup packet that may
+   * target this interface (interface-specific requests) or one of its
    * endpoints (endpoint-specific requests). The interface should examine
-   * the setup packet and handle any requests it recognizes.
+   * the setup packet and handle any requests it recognizes. If the setup packet
+   * is not for this interface, this API must return false.
+   *
+   * The interface must handle:
    *
-   * The interface may handle:
    * - Standard requests specific to this interface type
    * - Class-specific requests defined by the interface's USB class
    * - Vendor-specific requests for custom functionality
    * - Endpoint-specific requests for its endpoints
-   * - Anything that isn't a device or configuration level request
+   *
+   * The `endpoint_io` object is constructed by the enumerator to provide
+   * limited access to the control endpoint.
    *
    * If the request has a data phase (wLength > 0), use the endpoint I/O
-   * interface to send response data (device-to-host) or receive data
-   * (host-to-device).
+   * interface to send data from device-to-host via the `write` API or
+   * receive data from host-to-device via the `read` API.
+   *
+   * When a new setup packet is received from the HOST, the enumerator ensures
+   * that `hal::operation_not_permitted` is thrown from all `endpoint_io` APIs.
+   * Implementations of `driver_handle_request` MUST NOT catch this exception;
+   * it must propagate to the enumerator for handling. Correct use of RAII to
+   * undo state changes is advised. If cleanup requires a catch block, the
+   * exception must be rethrown via `throw;` before exiting.
    *
    * @param p_setup - Setup request from the host.
-   * @param p_ep_req - endpoint I/O interface for reading or writing data
-   * during the data phase of the control transfer.
+   * @param p_endpoint - endpoint I/O interface for reading or writing data to
+   * the host via the control endpoint during the data phase of the control
+   * transfer.
    * @return true - if the request was handled by the interface.
    * @return false - if the request could not be handled by interface.
+   * @throws hal::operation_not_permitted - If a new setup packet is received
+   * before the current control transfer completes. This exception must not be
+   * caught by the implementation; it is intended for the enumerator.
    */
-  bool handle_request(setup_packet const& p_setup, endpoint_io& p_ep_req)
+  bool handle_request(setup_packet const& p_setup, endpoint_io& p_endpoint)
   {
-    return driver_handle_request(p_setup, p_ep_req);
+    return driver_handle_request(p_setup, p_endpoint);
   }
 
   virtual ~interface() = default;
 
 private:
-  virtual descriptor_count driver_write_descriptors(descriptor_start p_start,
-                                                    endpoint_io& p_ep_req) = 0;
+  virtual descriptor_count driver_write_descriptors(
+    descriptor_start p_start,
+    endpoint_io& p_endpoint) = 0;
   virtual bool driver_write_string_descriptor(u8 p_index,
-                                              endpoint_io& p_ep_req) = 0;
+                                              endpoint_io& p_endpoint) = 0;
   virtual bool driver_handle_request(setup_packet const& p_setup,
-                                     endpoint_io& p_ep_req) = 0;
+                                     endpoint_io& p_endpoint) = 0;
 };
 }  // namespace hal::v5::usb
 

v4/tests/usb.test.cpp

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include <array>
+#include <optional>
 
 #include <libhal/error.hpp>
 #include <libhal/scatter_span.hpp>
@@ -62,6 +63,7 @@ class mock_usb_control_endpoint : public control_endpoint
   usize m_read_result{ 0 };
   callback<void(on_receive_tag)> m_receive_callback{};
   bool m_on_receive_called{ false };
+  std::optional<bool> m_has_setup_state{};
 
 private:
   [[nodiscard]] endpoint_info driver_info() const override
@@ -106,6 +108,11 @@ class mock_usb_control_endpoint : public control_endpoint
   {
     m_endpoint.reset();
   }
+
+  std::optional<bool> driver_has_setup() const noexcept override
+  {
+    return m_has_setup_state;
+  }
 };
 
 // Mock implementation for usb_in_endpoint
@@ -366,6 +373,57 @@ boost::ut::suite<"usb_control_endpoint_test"> control_endpoint_test = []() {
     endpoint.reset();
     expect(that % endpoint.m_endpoint.m_reset_called);
   };
+
+  "mock_usb_control_endpoint has_setup test"_test = []() {
+    mock_usb_control_endpoint endpoint;
+    endpoint.m_has_setup_state = std::nullopt;
+    auto const val1 = endpoint.has_setup();
+    expect(std::nullopt == val1);
+
+    endpoint.m_has_setup_state = true;
+    auto const val2 = endpoint.has_setup();
+    expect(that % true == *val2);
+
+    endpoint.m_has_setup_state = false;
+    auto const val3 = endpoint.has_setup();
+    expect(that % false == *val3);
+  };
+
+  "mock_usb_control_endpoint default has_setup test"_test = []() {
+    struct test_ctrl_ep : public hal::usb::control_endpoint
+    {
+      void driver_connect(bool) override
+      {
+      }
+      void driver_set_address(u8) override
+      {
+      }
+      void driver_write(scatter_span<byte const>) override
+      {
+      }
+      usize driver_read(scatter_span<byte>) override
+      {
+        return 0;
+      }
+      void driver_on_receive(callback<void(on_receive_tag)> const&) override
+      {
+      }
+      [[nodiscard]] endpoint_info driver_info() const override
+      {
+        return {};
+      }
+      void driver_stall(bool) override
+      {
+      }
+      void driver_reset() override
+      {
+      }
+      // driver_has_setup skipped!
+    } endpoint;
+
+    auto const val1 = endpoint.has_setup();
+    expect(std::nullopt == val1);
+  };
 };
 
 // Test for usb_in_endpoint interface