Skip to content

feat: Add machine/usb package with USB device class support #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xushiwei merged 2 commits into from
Nov 28, 2025
Merged

feat: Add machine/usb package with USB device class support #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c8a720
chore: Adapt import paths for machine/_usb package
luoliwoshang Nov 26, 2025
36b0a1a
refactor: Rename machine/_usb to machine/usb and update all imports
luoliwoshang Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
0 machine/_usb/adc/doc.go → machine/usb/adc/doc.go
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion machine/_usb/adc/midi/buffer.go → machine/usb/adc/midi/buffer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package midi

import (
"runtime/volatile"
"github.com/goplus/emb/runtime/volatile"
)

const bufferSize = 128
Expand Down
0 machine/_usb/adc/midi/messages.go → machine/usb/adc/midi/messages.go
View file
Open in desktop
File renamed without changes.
6 changes: 3 additions & 3 deletions machine/_usb/adc/midi/midi.go → machine/usb/adc/midi/midi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package midi

import (
"machine"
"machine/usb"
"machine/usb/descriptor"
"github.com/goplus/emb/machine"
Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Quality: Global Singleton Without Thread Safety

The global Midi variable is initialized in init() without synchronization. While this may work in single-threaded embedded contexts, it's not future-proof for concurrent access.

Recommendation:
Either:

  1. Use sync.Once for lazy initialization if concurrent access is possible
  2. Document clearly that this package assumes single-threaded execution
  3. Add build constraints if this is architecture-specific

"github.com/goplus/emb/machine/usb"
"github.com/goplus/emb/machine/usb/descriptor"
)

const (
Expand Down
0 machine/_usb/adc/midi/notes.go → machine/usb/adc/midi/notes.go
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion machine/_usb/cdc/buffer.go → machine/usb/cdc/buffer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package cdc

import (
"runtime/volatile"
"github.com/goplus/emb/runtime/volatile"
Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: Race Condition in Ring Buffer

The Put() operation is not atomic and lacks proper synchronization. Between checking Used() and updating the buffer, an interrupt could modify the buffer state, leading to data corruption.

Issues:

  1. Used() result can become stale between check at line 30 and buffer update at line 32
  2. No interrupt masking around the critical section
  3. Similar pattern exists in HID and MIDI ring buffers

Recommendation:
Use interrupt.Disable()/Restore() around critical sections (CDC code already uses this pattern elsewhere in usbcdc.go):

func (rb *rxRingBuffer) Put(val byte) bool {
    state := interrupt.Disable()
    defer interrupt.Restore(state)
    
    if rb.Used() != rxRingBufferSize {
        rb.head.Set(rb.head.Get() + 1)
        rb.buffer[rb.head.Get()%rxRingBufferSize].Set(val)
        return true
    }
    return false
}

Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance: Redundant Volatile Register Reads

Each rb.head.Get() is an expensive volatile register read. Line 31-32 performs this twice.

Optimization for embedded systems:

newHead := rb.head.Get() + 1
rb.head.Set(newHead)
rb.buffer[newHead%rxRingBufferSize].Set(val)

Further optimization: Since rxRingBufferSize = 128 is a power of 2, use bitwise AND instead of modulo (much faster on ARM Cortex-M without hardware divider):

const rxRingBufferMask = rxRingBufferSize - 1
rb.buffer[newHead&rxRingBufferMask].Set(val)

)

const rxRingBufferSize = 128
Expand Down
0 machine/_usb/cdc/cdc.go → machine/usb/cdc/cdc.go
View file
Open in desktop
File renamed without changes.
0 machine/_usb/cdc/doc.go → machine/usb/cdc/doc.go
View file
Open in desktop
File renamed without changes.
6 changes: 3 additions & 3 deletions machine/_usb/cdc/usbcdc.go → machine/usb/cdc/usbcdc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package cdc

import (
"errors"
"machine"
"machine/usb"
"runtime/interrupt"
"github.com/goplus/emb/machine"
Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMPORTANT: Silent Error Suppression

Errors from ReadByte() are silently ignored, potentially reading zero values instead of actual data.

Recommendation:
Either propagate the error or document why it can be safely ignored:

for i := 0; i < size; i++ {
    v, err := usbcdc.ReadByte()
    if err != nil {
        return i, err
    }
    data[i] = v
}

"github.com/goplus/emb/machine/usb"
"github.com/goplus/emb/runtime/interrupt"
)

var (
Expand Down
0 machine/_usb/config.go → machine/usb/config.go
View file
Open in desktop
File renamed without changes.
0 machine/_usb/descriptor/cdc.go → machine/usb/descriptor/cdc.go
View file
Open in desktop
File renamed without changes.
0 machine/_usb/descriptor/classspecific.go → machine/usb/descriptor/classspecific.go
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion machine/_usb/descriptor/configuration.go → machine/usb/descriptor/configuration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package descriptor

import (
"internal/binary"
"github.com/goplus/emb/internal/binary"
)

const (
Expand Down
2 changes: 1 addition & 1 deletion machine/_usb/descriptor/descriptor.go → machine/usb/descriptor/descriptor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package descriptor

import (
"runtime/volatile"
"github.com/goplus/emb/runtime/volatile"
)

const (
Expand Down
2 changes: 1 addition & 1 deletion machine/_usb/descriptor/device.go → machine/usb/descriptor/device.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package descriptor

import (
"internal/binary"
"github.com/goplus/emb/internal/binary"
)

const (
Expand Down
0 machine/_usb/descriptor/doc.go → machine/usb/descriptor/doc.go
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion machine/_usb/descriptor/endpoint.go → machine/usb/descriptor/endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package descriptor

import (
"internal/binary"
"github.com/goplus/emb/internal/binary"
)

/* Endpoint Descriptor
Expand Down
4 changes: 2 additions & 2 deletions machine/_usb/descriptor/hid.go → machine/usb/descriptor/hid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package descriptor

import (
"errors"
"internal/binary"
"internal/bytealg"
"github.com/goplus/emb/internal/binary"
Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance: Redundant Volatile Register Reads

Each call to rb.head.Get() is an expensive volatile register read. The value is fetched twice here (lines 31-32 in buffer implementations).

Optimization for embedded systems:

newHead := rb.head.Get() + 1
rb.head.Set(newHead)
rb.buffer[newHead%rxRingBufferSize].Set(val)

Additionally, since buffer sizes are powers of 2, use bitwise AND instead of modulo:

const rxRingBufferMask = rxRingBufferSize - 1
rb.buffer[newHead&rxRingBufferMask].Set(val)  // Much faster on ARM Cortex-M

"github.com/goplus/emb/internal/bytealg"
)

var configurationCDCHID = [configurationTypeLen]byte{
Expand Down
0 machine/_usb/descriptor/hidreport.go → machine/usb/descriptor/hidreport.go
View file
Open in desktop
File renamed without changes.
0 machine/_usb/descriptor/interface.go → machine/usb/descriptor/interface.go
View file
Open in desktop
File renamed without changes.
0 ...e/_usb/descriptor/interfaceassociation.go → ...ne/usb/descriptor/interfaceassociation.go
View file
Open in desktop
File renamed without changes.
0 machine/_usb/descriptor/joystick.go → machine/usb/descriptor/joystick.go
View file
Open in desktop
File renamed without changes.
0 machine/_usb/descriptor/midi.go → machine/usb/descriptor/midi.go
View file
Open in desktop
File renamed without changes.
0 machine/_usb/descriptor/msc.go → machine/usb/descriptor/msc.go
View file
Open in desktop
File renamed without changes.
0 machine/_usb/doc.go → machine/usb/doc.go
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion machine/_usb/hid/buffer.go → machine/usb/hid/buffer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package hid

import (
"runtime/volatile"
"github.com/goplus/emb/runtime/volatile"
)

const bufferSize = 128
Expand Down
0 machine/_usb/hid/doc.go → machine/usb/hid/doc.go
View file
Open in desktop
File renamed without changes.
6 changes: 3 additions & 3 deletions machine/_usb/hid/hid.go → machine/usb/hid/hid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package hid

import (
"errors"
"machine"
"machine/usb"
"machine/usb/descriptor"
"github.com/goplus/emb/machine"
"github.com/goplus/emb/machine/usb"
"github.com/goplus/emb/machine/usb/descriptor"
)

// from usb-hid.go
Expand Down
8 changes: 4 additions & 4 deletions machine/_usb/hid/joystick/joystick.go → machine/usb/hid/joystick/joystick.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package joystick

import (
"machine"
"machine/usb"
"machine/usb/descriptor"
"machine/usb/hid"
"github.com/goplus/emb/machine"
"github.com/goplus/emb/machine/usb"
"github.com/goplus/emb/machine/usb/descriptor"
"github.com/goplus/emb/machine/usb/hid"
)

var Joystick *joystick
Expand Down
2 changes: 1 addition & 1 deletion machine/_usb/hid/joystick/state.go → machine/usb/hid/joystick/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package joystick

import (
"machine/usb/descriptor"
"github.com/goplus/emb/machine/usb/descriptor"

"encoding/binary"
)
Expand Down
4 changes: 2 additions & 2 deletions machine/_usb/hid/keyboard/keyboard.go → machine/usb/hid/keyboard/keyboard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package keyboard

import (
"errors"
"machine"
"machine/usb/hid"
"github.com/goplus/emb/machine"
"github.com/goplus/emb/machine/usb/hid"
)

// from usb-hid-keyboard.go
Expand Down
0 machine/_usb/hid/keyboard/keycode.go → machine/usb/hid/keyboard/keycode.go
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions machine/_usb/hid/mouse/mouse.go → machine/usb/hid/mouse/mouse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package mouse

import (
"machine"
"machine/usb/hid"
"github.com/goplus/emb/machine"
"github.com/goplus/emb/machine/usb/hid"
)

var Mouse *mouse
Expand Down
4 changes: 2 additions & 2 deletions machine/_usb/msc/cbw.go → machine/usb/msc/cbw.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package msc

import (
"encoding/binary"
"machine/usb/msc/csw"
"machine/usb/msc/scsi"
"github.com/goplus/emb/machine/usb/msc/csw"
"github.com/goplus/emb/machine/usb/msc/scsi"
)

const (
Expand Down
0 machine/_usb/msc/csw/csw.go → machine/usb/msc/csw/csw.go
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion machine/_usb/msc/disk.go → machine/usb/msc/disk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"encoding/binary"
"errors"
"fmt"
"machine"
"github.com/goplus/emb/machine"
"time"
)

Expand Down
10 changes: 5 additions & 5 deletions machine/_usb/msc/msc.go → machine/usb/msc/msc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package msc

import (
"machine"
"machine/usb"
"machine/usb/descriptor"
"machine/usb/msc/csw"
"machine/usb/msc/scsi"
"github.com/goplus/emb/machine"
"github.com/goplus/emb/machine/usb"
Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance: Inefficient Busy-Wait Goroutine

This goroutine wakes 10,000 times per second to check a single boolean flag.

Issues:

  1. Continuous wakeup prevents deep sleep states (critical for battery-powered devices)
  2. Adds scheduler pressure in resource-constrained embedded systems
  3. Goroutine stack (~2KB minimum) permanently allocated

Recommendation:
Use channel-based signaling for event notification:

type msc struct {
    // ... existing fields ...
    taskChan chan struct{}
}

func (m *msc) processTasks() {
    for range m.taskChan {
        cmd := m.cbw.SCSICmd()
        // ... process task ...
        m.queuedBytes = 0
        m.taskQueued = false
        machine.AckUsbOutTransfer(usb.MSC_ENDPOINT_OUT)
    }
}

// In rxHandler, send signal: select { case m.taskChan <- struct{}{}: default: }

"github.com/goplus/emb/machine/usb/descriptor"
"github.com/goplus/emb/machine/usb/msc/csw"
"github.com/goplus/emb/machine/usb/msc/scsi"
"time"
)

Expand Down
6 changes: 3 additions & 3 deletions machine/_usb/msc/scsi.go → machine/usb/msc/scsi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package msc

import (
"encoding/binary"
"machine/usb"
"machine/usb/msc/csw"
"machine/usb/msc/scsi"
"github.com/goplus/emb/machine/usb"
"github.com/goplus/emb/machine/usb/msc/csw"
"github.com/goplus/emb/machine/usb/msc/scsi"
)

func (m *msc) scsiCmdBegin() {
Expand Down
0 machine/_usb/msc/scsi/scsi.go → machine/usb/msc/scsi/scsi.go
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions machine/_usb/msc/scsi_inquiry.go → machine/usb/msc/scsi_inquiry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package msc

import (
"encoding/binary"
"machine/usb/msc/csw"
"machine/usb/msc/scsi"
"github.com/goplus/emb/machine/usb/msc/csw"
"github.com/goplus/emb/machine/usb/msc/scsi"
)

type vpdPage struct {
Expand Down
4 changes: 2 additions & 2 deletions machine/_usb/msc/scsi_readwrite.go → machine/usb/msc/scsi_readwrite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package msc

import (
"errors"
"machine/usb/msc/csw"
"machine/usb/msc/scsi"
"github.com/goplus/emb/machine/usb/msc/csw"
"github.com/goplus/emb/machine/usb/msc/scsi"
)

var invalidWriteError = errors.New("invalid write offset or length")
Expand Down
4 changes: 2 additions & 2 deletions machine/_usb/msc/scsi_unmap.go → machine/usb/msc/scsi_unmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package msc

import (
"encoding/binary"
"machine/usb/msc/csw"
"machine/usb/msc/scsi"
"github.com/goplus/emb/machine/usb/msc/csw"
"github.com/goplus/emb/machine/usb/msc/scsi"
)

type Error int
Expand Down
6 changes: 3 additions & 3 deletions machine/_usb/msc/setup.go → machine/usb/msc/setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package msc

import (
"machine"
"machine/usb"
"machine/usb/msc/csw"
"github.com/goplus/emb/machine"
"github.com/goplus/emb/machine/usb"
"github.com/goplus/emb/machine/usb/msc/csw"
)

func setupPacketHandler(setup usb.Setup) bool {
Expand Down
0 machine/_usb/usb.go → machine/usb/usb.go
View file
Open in desktop
File renamed without changes.
Loading