> On Sept. 12, 2012, 2:20 p.m., Alex Fiestas wrote: > > From where I stand, I see HAL as the official backend for freeBSD, no > > modern linux distro (or any distro that will update their libsolid version) > > is shipping with HAL. > > > > So, if with this patch freeBSD support is better, please go ahead! > > Alberto Villa wrote: > Andriy Gapon submitted an equivalent review request time ago: > https://git.reviewboard.kde.org/r/105432 > It solves the same issue (I was not aware of it), introducing a function > which was proposed for the Solid API. Apart from the specific code, for which > you seem to trust our judgement, what's your preferred approach? I admit that > at this very moment I am not aware of other places which require that > function, but I guess there could be some (I may have seen one where CD > drives need to address their parent storage drive). > > By the way, would you approve a change to set both individual Removable > and Hotpluggable properties? This would make the dataengine more consistent, > while it's the plasmoid which has to check for both the properties to > identify removable devices (I've read the HAL spec - which I think can be a > reliable reference for those properties - and yes, one should not be set when > the other one is, so we're handling them in the wrong way). > > Alberto Villa wrote: > > I admit that at this very moment I am not aware of other places which > require that function > > OK, now I know of at least another place. > > Alex Fiestas wrote: > About the properties I think you are correct and this patch should go on > (btw we don't maintain that datanegine, that's plasma work). > > About the method, It totally makes sense but we will have to wait until > libsolid2 at least if we want it public (I may accept a version of it > private). > > Good job btw :) > >
I don't find a private version useful at the moment, so I committed the patch with the local function. About the method, I hope to see it in libsolid2. :) - Alberto ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106378/#review18899 ----------------------------------------------------------- On Sept. 12, 2012, 5:07 p.m., Alberto Villa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106378/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2012, 5:07 p.m.) > > > Review request for Solid. > > > Description > ------- > > Current hack to check for Removable property in StorageAccess devices goes up > only one level to search for the StorageDrive device. This works fine with > UDev, but not with HAL, which can have (at least on FreeBSD, where I'm > testing it) a StorageVolume device in the middle. Going up the whole tree of > the Block device ensures that we eventually get to the StorageDrive one to > fetch the correct Removable property. > > While here, I'd like to set both Removable and Hotpluggable properties (as > done few lines above), as they are exclusive and have a very different > meaning (Removable being stuff that can be removed while its device node > survives, like CDs and floppies, and Hotpluggable being stuff like USB and > eSATA devices, which can be removed while the system is running without > leaving behind a stale device node). Plasmoids using the dataengine should > check for both the properties, as they shouldn't be defined at the same time > (we're using them in the wrong way, according to the HAL spec which, I guess, > inspired the Solid interface; it's not a big problem, but the code should > handle the correct usage too, as a future devd backend might follow it). > > If approved, I'd like to commit this also to 4.9. > > > Diffs > ----- > > plasma/generic/dataengines/soliddevice/soliddeviceengine.cpp 86f123c > > Diff: http://git.reviewboard.kde.org/r/106378/diff/ > > > Testing > ------- > > Tested successfully on FreeBSD 10-CURRENT r239665 with KDE SC 4.9.0: my USB > flash drives now appear in the device notifier plasmoid (and a console.log() > in the plasmoid itself confirms that the device now has the Removable > property). > > I also double-checked with a UDev-backed `solid-hardware list details` log > that the logic correctly applies to UDev. > > > Thanks, > > Alberto Villa > >
_______________________________________________ Kde-hardware-devel mailing list Kde-hardware-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-hardware-devel
