✨ (minor) Usb enumeration #92

PhazonicRidley · 2026-01-30T00:17:51Z

Implemented a USB enumerator that can be used in various projects. This is meant as a default and reference implementation of an enumerator using the libhal USB system.

kammce

Please rebase your project and fix your merge conflicts. Also, we moved the libhal-util code into v5 in preperation for v6 which will use libhal-v5. So we can remove the include files within the include directory and only keep the ones within v5/*

.github/workflows/ci.yml

include/libhal-util/usb/descriptors.hpp

include/libhal-util/usb/endpoints.hpp

For devices that use the libhal usb interface high level class. Handles configuration at the device and usb configuration level Handles multiple configurations and multiple interfaces.

Simulated host by sending enumerator requests and having it respond. Correct payloads were verified

Changed usb.hpp to be a shorthand for including all usb utils

For devices that use the libhal usb interface high level class. Handles configuration at the device and usb configuration level Handles multiple configurations and multiple interfaces.

Simulated host by sending enumerator requests and having it respond. Correct payloads were verified

Changed usb.hpp to be a shorthand for including all usb utils

kammce

I haven't reviewed everything. I'm posting what I have now because I'm not sure if github will lose my info.

.claude/settings.local.json

kammce · 2026-02-09T20:42:22Z

v5/include/libhal-util/mock/usb.hpp

+#include <libhal/units.hpp>
+#include <libhal/usb.hpp>
+
+#include "libhal-util/usb/utils.hpp"


Why not use the standard <> for this?

I can, I left it as "" because its the same repo that I am referencing and I am not a huge fan of relative path includes.

kammce · 2026-02-09T20:56:42Z

v5/include/libhal-util/mock/usb.hpp

+  std::array<u8, 9> m_packed_array = {
+    iface_desc_length,
+    iface_desc_type,
+    0,  // interface_number
+    0,  // alternate_setting
+    1,  //  num_endpoints
+    2,  //  interface_class
+    3,  // interface_sub_class
+    4,  // interface_protocol
+    0   // interface name index
+  };


We should be using the constexpr length as the template argument and using the object's size to set the first element.

See this example of this working: https://godbolt.org/z/1j3rqzr1W

The code change would look like this:

Suggested change

std::array<u8, 9> m_packed_array = {

iface_desc_length,

iface_desc_type,

0, // interface_number

0, // alternate_setting

1, // num_endpoints

2, // interface_class

3, // interface_sub_class

4, // interface_protocol

0 // interface name index

};

std::array<u8, iface_desc_length> m_packed_array = {

sizeof(m_packed_array),

iface_desc_type,

0, // interface_number

0, // alternate_setting

1, // num_endpoints

2, // interface_class

3, // interface_sub_class

4, // interface_protocol

0 // interface name index

};

Now you don't have to worry about accidentially making a mistake which causes the sizes to be off. Like for example, accidentally putting 10 or 19 instead of 9 for the length.

kammce · 2026-02-09T20:57:34Z

v5/include/libhal-util/mock/usb.hpp

+constexpr u8 iface_desc_length = 9;
+constexpr u8 iface_desc_type = 0x4;
+constexpr u8 str_desc_type = 0x3;
+constexpr u8 iad_length = 0x08;
+constexpr u8 iad_type = 0x0B;


I'd prefer these written out fully. interface and string and I'm not sure what iad stands for off the top of my head, so spelling this out would be helpful

Even for just a mock?

kammce · 2026-02-09T20:59:27Z

v5/include/libhal-util/mock/usb.hpp

+template<typename T>
+bool span_eq(std::span<T> const lhs, std::span<T> const rhs)
+{
+  if (lhs.size() != rhs.size()) {
+    return false;
+  }
+
+  for (size_t i = 0; i < lhs.size(); i++) {
+    if (lhs[i] != rhs[i]) {
+      return false;
+    }
+  }
+  return true;
+}
+
+template<typename T>
+bool span_ne(std::span<T> const lhs, std::span<T> const rhs)
+{
+  return !(lhs == rhs);
+}


Does std::ranges::equals(lhs, rhs) not work in this situation?

kammce · 2026-02-09T23:05:11Z

v5/include/libhal-util/usb/enumerator.hpp

+    strong_ptr<std::array<configuration, num_configs>> const& p_configs,
+    u16 p_lang_str,  // NOLINT
+    u8 p_starting_str_idx,
+    bool enumerate_immediately = true)


NIT: I'm not a fan of the bool parameter here. And this is a pretty large number of parameters, so this looks like a good option for a struct parameter.

kammce · 2026-02-09T23:07:46Z

v5/include/libhal-util/usb/enumerator.hpp

+    strong_ptr<device> const& p_device,
+    strong_ptr<std::array<configuration, num_configs>> const& p_configs,
+    u16 p_lang_str,  // NOLINT
+    u8 p_starting_str_idx,


Why should we allow the developer to choose this and why is it required? Is there some feature that gets unlocked from this depending on what value is set? Otherwise, this is laying a trap for someone when used. If you set this too low, they get an error. Set it too high, get an error. Or we can automate this and remove the error path completely.

kammce · 2026-02-09T23:14:01Z

v5/include/libhal-util/usb/descriptors.hpp

+  }
+
+  std::u16string_view name;
+  std::pmr::vector<strong_ptr<interface>> interfaces;


Consider using hal::allocated_buffer<strong_ptr<interface>> instead of vector. We don't need to resize the array after construction, and thus all of its code needed to handle pushing and resizing can be eliminated. This way we pay for only what we needed which is a pointer, a length, and the allocator that created.

#include <libhal/allocated_buffer.hpp>

It was meant for just buffers but its usable for any type. (maybe I should make an alias for it thats dynamic_array).

kammce · 2026-02-10T00:14:54Z

v5/include/libhal-util/usb/enumerator.hpp

+    if (enumerate_immediately) {
+      enumerate();
+    }


I'm not sure how I feel about this. My current opinion is that we should remove this as the operation isn't very async at the moment. Its like calling a scheduler and not expecting to get back control of the code beyond that point if the usb host is not connected.

kammce · 2026-02-10T00:34:41Z

v5/include/libhal-util/usb/enumerator.hpp

+    }
+  }
+
+  void enumerate()


This is purely a suggestion but I think we might benefit from this rename:

Suggested change

void enumerate()

void enumerate_sync_prototype0()

Why? We can provide this temporary API that we plan to deprecate in the future. This way we can work with the experimental stuff while preventing the API from being broken. This will allow us to publish the code and leave space for a better implementation in the future. Because making this async may require a breaking API change. Or a new API called enumerate2().

PhazonicRidley changed the title ~~✨ Usb enumeration~~ ✨ (minor) Usb enumeration Jan 30, 2026

PhazonicRidley force-pushed the usb-enumeration branch from d2cc07b to c8f1daa Compare January 30, 2026 00:27

PhazonicRidley marked this pull request as draft February 1, 2026 05:19

PhazonicRidley force-pushed the usb-enumeration branch 2 times, most recently from 780ef28 to f41be54 Compare February 5, 2026 20:57

kammce self-requested a review February 7, 2026 22:59

kammce requested changes Feb 8, 2026

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

include/libhal-util/usb/descriptors.hpp Outdated Show resolved Hide resolved

include/libhal-util/usb/endpoints.hpp Outdated Show resolved Hide resolved

PhazonicRidley force-pushed the usb-enumeration branch 3 times, most recently from 6b474f8 to c9c62f1 Compare February 9, 2026 19:10

PhazonicRidley marked this pull request as ready for review February 10, 2026 00:26

PhazonicRidley requested a review from kammce February 10, 2026 00:26

PhazonicRidley and others added 17 commits February 9, 2026 16:28

✨ feat(usb_enumeration): Made an enumerator

ce7b663

For devices that use the libhal usb interface high level class. Handles configuration at the device and usb configuration level Handles multiple configurations and multiple interfaces.

🏷️ Updated usb utils to use libhal 4.18.0+ APIs

e655173

✅ Tested enumerator

4cf652b

Simulated host by sending enumerator requests and having it respond. Correct payloads were verified

♻️ Moved endpoint utils to their own file

767b305

Changed usb.hpp to be a shorthand for including all usb utils

🐛 Enumerator wait loop got optmized out, volatile prevents this

67a1e4c

🐛 Fixed logic to have enumeration work on real hardware

033e5d7

📝 Document descriptors and enumerator class

1a03390

♻️ Move source code to v5/ directory (libhal#82)

2bef819

✨ feat(usb_enumeration): Made an enumerator

a89d57a

For devices that use the libhal usb interface high level class. Handles configuration at the device and usb configuration level Handles multiple configurations and multiple interfaces.

✅ Tested enumerator

b1c0e04

Simulated host by sending enumerator requests and having it respond. Correct payloads were verified

♻️ Moved endpoint utils to their own file

778857c

Changed usb.hpp to be a shorthand for including all usb utils

🐛 Enumerator wait loop got optmized out, volatile prevents this

6677c55

🐛 Fixed logic to have enumeration work on real hardware

c04d212

♻️ Refactored USB utils into their own folder and namespace

fd3074d

🧑‍💻 Updated to support new API for setup_packet in libhal

70b0405

🧑‍💻 Updated to support new API for setup_packet in libhal

50ebee4

🚚 Restructured files and APIs to align with latest libhal

ff91bbf

PhazonicRidley force-pushed the usb-enumeration branch from c9c62f1 to ff91bbf Compare February 10, 2026 00:29

kammce requested changes Feb 10, 2026

View reviewed changes

✨ (minor) Usb enumeration #92

Are you sure you want to change the base?

✨ (minor) Usb enumeration #92

Uh oh!

Conversation

PhazonicRidley commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kammce left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kammce left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PhazonicRidley commented Jan 30, 2026 •

edited

Loading