Add DeviceId from str conversion#348
Conversation
e796108 to
c75ca79
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #348 +/- ##
==========================================
+ Coverage 46.14% 46.44% +0.30%
==========================================
Files 28 28
Lines 2659 2676 +17
Branches 2659 2676 +17
==========================================
+ Hits 1227 1243 +16
Misses 1375 1375
- Partials 57 58 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
| } | ||
|
|
||
| impl TryFrom<&str> for DeviceId { |
There was a problem hiding this comment.
How about implementing FromStr instead? That's more common for parsing things from a string.
| fn from_string() { | ||
| let device_id = DeviceId::try_from("hci0/dev_11_22_33_44_55_66"); | ||
| assert_eq!(device_id.unwrap().to_string(), "hci0/dev_11_22_33_44_55_66"); | ||
| } |
There was a problem hiding this comment.
Please add a test for converting an invalid path String to a DeviceId too, to make sure it returns an error as expected.
There was a problem hiding this comment.
actually i did some digging and turn out that dbus::Path returns Result<Path, Infallible> so it always succeeds and should never produce an error, so i ended up with adding simple validation (check if input starts with hci)
There was a problem hiding this comment.
It looks to me that Path::try_from is an auto implementation based on Path::from, which in turn will panic if there is an invalid path. But Path::new will return an error.
|
|
||
| fn try_from(s: &str) -> Result<Self, Self::Error> { | ||
| let path = Path::try_from(format!("/org/bluez/{}", s)) | ||
| .map_err(|_| dbus::Error::new_failed("Invalid D-Bus path"))?; |
There was a problem hiding this comment.
Why do you need to map the error here?
c75ca79 to
e4200f5
Compare
adding missing conversion from
&strresolve: #296
use case (see btleplug feature)
DeviceIdas string in databaseBaluetoothSession.get_device_info(restored_id)alternatively restore btleplug PeripheralId (wrapper for DeviceId) which i believe is referred in related issue