Skip to content
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.

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

fixed CH546 timeout issue #12

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

Sandr0x00
Copy link

@Sandr0x00 Sandr0x00 commented Jan 12, 2025

Fixes #11 and jnweiger#51.

Summary by Sourcery

Bug Fixes:

  • Resolve timeout issues occurring when writing data to the badge by ensuring both input and output endpoints are correctly handled.

Copy link

sourcery-ai bot commented Jan 12, 2025

Reviewer's Guide by Sourcery

This pull request fixes a timeout issue by correctly handling the IN endpoint of the USB device. It modifies the lednamebadge.py file to open and utilize both the IN and OUT endpoints during communication. This addresses the timeout problem reported in the linked issues.

Sequence diagram for USB communication with IN/OUT endpoints

sequenceDiagram
    participant App as LED Badge App
    participant OUT as USB OUT Endpoint
    participant IN as USB IN Endpoint
    participant Device as LED Badge Device

    Note over App,Device: New implementation with both endpoints
    App->>OUT: Write 64-byte chunk
    OUT->>Device: Send data
    Device->>IN: Send acknowledgment
    IN->>App: Read acknowledgment (64 bytes)
    Note over App,Device: Process repeats for each chunk
Loading

Class diagram showing WriteLibUsb changes

classDiagram
    class WriteLibUsb {
        -description
        -dev
        -endpoint_out
        -endpoint_in
        +get_name()
        +get_description()
        +_open(device_id)
        +close()
        +_get_available_devices()
        +is_ready()
        +_write(buf)
    }
    note for WriteLibUsb "Modified to handle both IN/OUT endpoints"
Loading

File-Level Changes

Change Details Files
Added handling for the IN endpoint of the USB device
  • Added endpoint_in member to the WriteLibUsb class.
  • Initialized and used the endpoint_in for reading data in the _open, close, _get_available_devices, and _write methods.
  • Modified device identification and description to include both endpoints.
  • Updated device discovery to find both IN and OUT endpoints and ensure both are present before adding the device to the list of available devices
lednamebadge.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @Sandr0x00 - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Please expand the commit message to explain the technical changes made (splitting USB endpoints and adding response reads) and how this fixes the timeout issue. This will help document the fix for future reference.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Sandr0x00
Copy link
Author

I actually don't know if this causes issues on other devices. Please someone test and report.
Thank you

@thesnable
Copy link

ping This. I tried the Sandro0x00 fork it works nice. Can we commit this to master? Thanks alot

@mariobehling mariobehling requested a review from kienvo March 18, 2025 11:25
@kienvo
Copy link
Member

kienvo commented Mar 18, 2025

I'm still having the same error with libusb. But hidapi works fine. I tried with CH583 badge (OEM firmware). I don't have any CH546 badge to test this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CH546 usb.core.USBTimeoutError: [Errno 110] Operation timed out Segmentation fault
3 participants