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

Fixing broken PID/VID #230

Merged
merged 3 commits into from
Nov 23, 2024
Merged

Fixing broken PID/VID #230

merged 3 commits into from
Nov 23, 2024

Conversation

Mystfit
Copy link
Contributor

@Mystfit Mystfit commented Oct 3, 2024

Upstream NimBLE version 1.4.2 allows us to set the PID, VID and GUID normally without needing to flip high/low bytes. Using PNPVersionField to preserve backwards compatibility since it was added in the same upstream commit. If there's a better way to detect the upstream NimBLE version then that would probably be a better solution.

Upstream NimBLE version 1.4.2 allows us to set the PID, VID and GUID normally without needing to flip high/low bytes. Using PNPVersionField to detect the version since it was added in the same commit.
@Mystfit
Copy link
Contributor Author

Mystfit commented Oct 3, 2024

Tested by running the CharacteristicsConfiguration example with VID set to 0xe502, PID set to 0xABCD and version GUID set to 0x1234.

First I ran the sketch with the old behaviour and the following string was returned in Windows device manager for the Hardware IDs property: BTHLEDevice{00001812-0000-1000-8000-00805f9b34fb}_Dev_VID&0102e5_PID&cdab_REV&3412
As you can see, the VID (ignoring first 01 byte), PID and REV properties have their first and second bytes flipped.

Then I re-ran the sketch with the fix applied that removes the VID, PID and GUID high/low bytes reversal and received the following: BTHLEDevice{00001812-0000-1000-8000-00805f9b34fb}_Dev_VID&01e502_PID&abcd_REV&1234 which matches the provided PID, VID and GUID values.

I also tested this fix over in my EPS32-BLE-CompositeHID fork with my Xbox controller sketch. The VID and PID are vital for convincing Windows to load the correct Bluetooth LE XINPUT compatible input device driver and without the fix, Windows saw my board as a generic gamepad but with the fix, it started working again.

@Mystfit Mystfit mentioned this pull request Oct 3, 2024
@lemmingDev
Copy link
Owner

lemmingDev commented Nov 23, 2024

Thanks for submitting this fix

Sorry for the delay

Should it be

vid = configuration.getVid();
pid = configuration.getPid();
guidVersion = configuration.getGuidVersion();

#ifndef PNPVersionField
            uint8_t high = highByte(vid);
            and so on...

instead of

#ifndef PNPVersionField
          vid = configuration.getVid();
          pid = configuration.getPid();
          guidVersion = configuration.getGuidVersion();

          uint8_t high = highByte(vid);
          and so on...

so that vid, pid and guidVersion are taken from the user if set, instead of always using the defaults in the case that PNPVersionField is defined?

Copy link
Owner

@lemmingDev lemmingDev left a comment

Choose a reason for hiding this comment

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

#ifndef PNPVersionField
needs to be moved under

vid = configuration.getVid();
pid = configuration.getPid();
guidVersion = configuration.getGuidVersion();

@lemmingDev
Copy link
Owner

Might be quicker for me to just manually add the 2 lines, though this would make you a contributor I guess ;)

Copy link
Owner

@lemmingDev lemmingDev left a comment

Choose a reason for hiding this comment

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

Nice

@lemmingDev lemmingDev merged commit 873167d into lemmingDev:master Nov 23, 2024
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.

2 participants