search

ruabmbua / hidapi-rs

Rust bindings for the hidapi C library
MIT License
172 stars 81 forks source link

Do not fail on wchar_t* to string conversion #53

Closed ruabmbua closed 4 years ago

ruabmbua commented 4 years ago

Quick fix for issue #52.

Instead of failing, return None value.

Edit:

Also added new DeviceInfo with access to raw wchar_t strings, and deprecated HidDeviceInfo and friends.

ruabmbua commented 4 years ago

@fpagliughi Can you try this fix?

lnicola commented 4 years ago

Should these be OsStrings instead?

ruabmbua commented 4 years ago

@lnicola Maybe. The problem is, OsString claims, it is the OS representation of strings, but the wchar_t in this case are HID / USB specific, so they don`t clearly match OsString. I could however export an interface with direct access to the wchar_t strings.

What do you think?

lnicola commented 4 years ago

Ah, I didn't realize that. https://docs.rs/widestring/0.4.0/widestring/ might be a solution here?

All four string types may still have unpaired UTF-16 surrogates or invalid UTF-32 code points; ill-formed data is preserved until conversion to a basic Rust String.

ruabmbua commented 4 years ago

Well, currently I use char::from_u32(), which should convert from any valid unicode codepoint. If it fails, its just not a valid codepoint. widestring won`t be able to do anything about that.

ruabmbua commented 4 years ago

Well, currently I use char::from_u32(), which should convert from any valid unicode codepoint. If it fails, its just not a valid codepoint. widestring won`t be able to do anything about that.

Ups, did not read your citation...

Still, I`d like to leave this to the user. I will add access to the underlying representation.

ruabmbua commented 4 years ago

Ahh all stuff in HidDeviceInfo is public... I do not want to break compatibility...

lnicola commented 4 years ago

I figured. We could use widestring now internally and do the string conversion (with to_string or to_string_lossy) and either expose separate widestring methods now (like env vs. env_os) or make the change in the next breaking version.

But maybe you think it's too much of a hassle?

ruabmbua commented 4 years ago

I think I will deprecate Hidapi::devices(), and make a new one which returns a iterator with a new DeviceInfo type. This time with getters.

ruabmbua commented 4 years ago

Ok, I added DeviceInfo as an alternative.

Its somewhat ugly internally (keeping around a compatibility, and new list). Can be removed after next major breaking change.