gh-libusb/hidapi
Watch
1
0
Fork
You've already forked hidapi
 0
Code Issues 58 Pull requests 18 Releases 13 Wiki Activity

README: Add warning about writing data to HID products #106

Merged
Qbicz merged 3 commits from sample-code-warning into master 2019-11-05 07:21:23 -06:00
Conversation 11 Commits 3 Files changed 1 +12 -2
Qbicz commented 2019-10-18 02:33:36 -05:00 (Migrated from github.com)

In response to https://github.com/libusb/hidapi/issues/105

In response to https://github.com/libusb/hidapi/issues/105
Qbicz commented 2019-10-18 02:34:22 -05:00 (Migrated from github.com)

@Youw @z3ntu @amosbird can you review?

@Youw @z3ntu @amosbird can you review?
z3ntu (Migrated from github.com) reviewed 2019-10-18 04:02:57 -05:00
README.md
@ -135,2 +139,4 @@
hid_close(handle);
// Finalize the hidapi library
res = hid_exit();
z3ntu (Migrated from github.com) commented 2019-10-18 04:02:56 -05:00

programs instead of programmes? I know it's a bit British vs American English but it's more fitting in this context.

`programs` instead of `programmes`? I know it's a bit British vs American English but it's more fitting in this context.
z3ntu (Migrated from github.com) reviewed 2019-10-18 04:03:56 -05:00
README.md
@ -68,12 +68,15 @@ which HIDAPI supports. Since it relies on a 3rd party library, building it
is optional but recommended because it is so useful when debugging hardware.
z3ntu (Migrated from github.com) commented 2019-10-18 04:03:56 -05:00

I would use *s instead of `s, so to make it bold because it looks a bit weird in this preformatted style.

I would use *s instead of `s, so to make it bold because it looks a bit weird in this preformatted style.
Youw (Migrated from github.com) reviewed 2019-10-18 09:24:38 -05:00
Youw (Migrated from github.com) left a comment

only against devices which firmware
you control

This statement is a bit too strong - you may not nesessary control the FW, but safely write, e.g. if you have official spec, etc.

I was thinking, maybe add explicit check for dev/ven before hid_write in the example

> only against devices which firmware you control This statement is a bit too strong - you may not nesessary control the FW, but safely write, e.g. if you have official spec, etc. I was thinking, maybe add explicit check for dev/ven before hid_write in the example
Qbicz (Migrated from github.com) reviewed 2019-10-21 12:49:09 -05:00
README.md
@ -135,2 +139,4 @@
hid_close(handle);
// Finalize the hidapi library
res = hid_exit();
Qbicz (Migrated from github.com) commented 2019-10-21 12:49:09 -05:00

applications ;)

`applications` ;)
Qbicz (Migrated from github.com) reviewed 2019-10-21 12:49:16 -05:00
README.md
@ -68,12 +68,15 @@ which HIDAPI supports. Since it relies on a 3rd party library, building it
is optional but recommended because it is so useful when debugging hardware.
Qbicz (Migrated from github.com) commented 2019-10-21 12:49:16 -05:00

done

done
Qbicz commented 2019-10-21 12:52:55 -05:00 (Migrated from github.com)

This statement is a bit too strong - you may not nesessary control the FW, but safely write, e.g. if you have official spec, etc.

@Youw done.

I was thinking, maybe add explicit check for dev/ven before hid_write in the example

I think that would be too much, since to write to the device in the sample you have to enumerate devices, check vid/pid and hardcode it to the sample. Or do you mean interactive check like with gets()?

> This statement is a bit too strong - you may not nesessary control the FW, but safely write, e.g. if you have official spec, etc. @Youw done. > I was thinking, maybe add explicit check for dev/ven before hid_write in the example I think that would be too much, since to write to the device in the sample you have to enumerate devices, check vid/pid and hardcode it to the sample. Or do you mean interactive check like with `gets()`?
Youw commented 2019-10-21 15:32:48 -05:00 (Migrated from github.com)

I was thinking something like:

#define SAMPLE_VEN_ID ...
#define SAMPLE_DEV_ID ...
unsigned short ven_id = SAMPLE_VEN_ID;
unsigned short dev_id = SAMPLE_DEV_ID;

int main(/*...*/) {
  if (argc /* ... */ ) {
    // ...
    ven_id = // parse arguments, etc.
    // ...
  }

  handle = hid_open(ven_id, dev_id, NULL);
  // ...

  // a WARNING comment here, what we're know what we're doing, but only with a specific device
  if (ven_id == SAMPLE_VEN_ID && dev_id == SAMPLE_DEV_ID) {
    // ...
    res = hid_write(handle, buf, 65);
    // ...
  }
  // hid_close
  // hid_exit
}

So we can include #14 and a little more, like explicitly encourage to hid_close which is not in an example.

Maybe this is a little too large change, but anyway nice to have.

I was thinking something like: ```cpp #define SAMPLE_VEN_ID ... #define SAMPLE_DEV_ID ... unsigned short ven_id = SAMPLE_VEN_ID; unsigned short dev_id = SAMPLE_DEV_ID; int main(/*...*/) { if (argc /* ... */ ) { // ... ven_id = // parse arguments, etc. // ... } handle = hid_open(ven_id, dev_id, NULL); // ... // a WARNING comment here, what we're know what we're doing, but only with a specific device if (ven_id == SAMPLE_VEN_ID && dev_id == SAMPLE_DEV_ID) { // ... res = hid_write(handle, buf, 65); // ... } // hid_close // hid_exit } ``` So we can include #14 and a little more, like explicitly encourage to `hid_close` which is not in an example. --- Maybe this is a little too large change, but anyway nice to have.
Youw (Migrated from github.com) approved these changes 2019-10-31 05:57:30 -05:00
Qbicz commented 2019-11-03 15:51:00 -06:00 (Migrated from github.com)

@Youw thanks for your comments, I added hid_close() call to the sample as you suggested, so this is now explicitly visible.

@Youw thanks for your comments, I added `hid_close()` call to the sample as you suggested, so this is now explicitly visible.
Youw (Migrated from github.com) approved these changes 2019-11-04 05:51:32 -06:00
Youw (Migrated from github.com) left a comment

and let's merge it after #115

and let's merge it after #115
README.md
@ -132,6 +135,9 @@ int main(int argc, char* argv[])
for (i = 0; i < 4; i++)
Youw (Migrated from github.com) commented 2019-11-04 05:49:25 -06:00 
You can also use [hidtest/test.c](hidtest/test.c)
```suggestion You can also use [hidtest/test.c](hidtest/test.c) ```
Sign in to join this conversation.
Reviewers
No reviewers
Youw
Labels
Clear labels   
3rdparty
3rd-party related issue/question

  
Android
Android OS related

  
API
API change, Version 1 stuff

  
Author does not respond

   
bsd
FreeBSD, NetBSD, OpenBSD, etc

  
bug
Something isn't working

  
build system/CI
Anything related to building the project or running on CI

  
ci-virtual-device
Run the virtual HID device CI jobs (Windows driver + libusb raw-gadget)

  
CMake
Related to CMake build system

  
Contributions Welcome
Any external help is welcome

  
Core
Related to common codes like hidapi.h

  
dependencies
Pull requests that update a dependency file

  
device-specific
Issues/PRs related to a specific device

  
documentation
Improvements or additions to documentation

  
don't_merge
Don't merge this PR as is

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
Example
Related to hidtest or new examples

  
Haiku
Related to Haiku OS (using libusb backend)

  
hidraw
Related to Linux/hidraw backend

  
hotplug
Related to hotplug

  
info required
Need more input from PR/issue author

  
invalid
Not a HIDAPI issue

  
libusb
Related to libusb backend

  
macOS
Related to macOS backend

  
Misc
Typo, whitespace, etc.

  
Not an issue

   
question
Information is requested

  
Release
Related to new release

  
Test
Related to built-in test (not hidtest)

  
Test GUI
Related to a Test GUI application

  
Windows
Related to Windows backend

  
wontfix
This will not be worked on

No labels
3rdparty
Android
API
Author does not respond
bsd
bug
build system/CI
ci-virtual-device
CMake
Contributions Welcome
Core
dependencies
device-specific
documentation
don't_merge
duplicate
enhancement
Example
Haiku
hidraw
hotplug
info required
invalid
libusb
macOS
Misc
Not an issue
question
Release
Test
Test GUI
Windows
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-libusb/hidapi!106
No description provided.