GNOME Bugzilla – Bug 509621
Implement obex-ftp backend
Last modified: 2008-02-28 14:32:11 UTC
We want an obex-ftp (bluetooth) backend.
Would it make sense to rely on the code written for obex-data-server[1]? Maybe some of it can be reused? [1] http://tadas.dailyda.com/blog/
I don't really know much about bluetooth, so I can't really comment. Better people to ask would be Richard Hult (he did some initial experimenting on a backend) and Bastien Nocera.
Richard, do you have that code somewhere?
We should be using obex-data-server to get the data from the devices: SVN: svn://svn.muiline.com/obex-data-server/trunk/ ViewVC: http://svn.muiline.com/cgi-bin/viewvc.cgi/obex-data-server/trunk/ Blog: http://tadas.dailyda.com/blog/ Fedora package: https://bugzilla.redhat.com/show_bug.cgi?id=428972 The API is described in dbus-api.txt in the root dir.
My efforts never really left the experimental stage and now I have zero time to work on it :/. What I did was the code to enumerate paired obex ftp devices through the bluez dbus api. I can probably dig that out and put somewhere. Does obex-data-server have an obex ftp client implementation? I haven't looked at it yet, just wondering since the name sounds more like a server side for obex push and/or ftp.
(In reply to comment #5) > My efforts never really left the experimental stage and now I have zero time to > work on it :/. What I did was the code to enumerate paired obex ftp devices > through the bluez dbus api. I can probably dig that out and put somewhere. Probably not needed right now then, as we use bluetooth-applet's device selection widget to choose the device to browse. > Does obex-data-server have an obex ftp client implementation? I haven't looked > at it yet, just wondering since the name sounds more like a server side for > obex push and/or ftp. It's "server" in the sense that it's a D-Bus service, and it does do client. The bluetooth-sendto in bluez-gnome uses that.
(In reply to comment #0) > We want an obex-ftp (bluetooth) backend. It would be nice if it would also support obex-ftp over usb.
(In reply to comment #7) > (In reply to comment #0) > > We want an obex-ftp (bluetooth) backend. > > It would be nice if it would also support obex-ftp over usb. This isn't a priority, as the gnome-vfs backend didn't support it, and obex-data-server doesn't support it yet either.
Created attachment 103568 [details] [review] gvfs-obexftp-1.patch First draft patch. - Doesn't allow listing obex:// (I think somebody else will have to work on that, as it's not visible in the UI, we use bluez-gnome to list devices) - Only supports mounting and listing files, although the enumeration hangs, and gvfs-ls never returns, that'll be one for tomorrow
Created attachment 103572 [details] [review] gvfs-obexftp-2.patch Sigh, forgot to call g_vfs_job_succeeded(). Listing works fine on '/'. Need to implement directory changing now.
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #0) > > > We want an obex-ftp (bluetooth) backend. > > > > It would be nice if it would also support obex-ftp over usb. > > This isn't a priority, as the gnome-vfs backend didn't support it, and > obex-data-server doesn't support it yet either. > I guess I should then file it as an enhancement in a new bug, right?
(In reply to comment #11) > I guess I should then file it as an enhancement in a new bug, right? Feel free.
Created attachment 103587 [details] [review] gvfs-obexftp-3.patch - enumeration works on all directories. - get-info works on all files and directories (except /) but doesn't include FS information (should be easy with the undocumented GetCapability, see http://bugs.muiline.com/view.php?id=37) TODO: - create directory - delete file/directory - get FS information Not possible right now: - rename/move (see http://bugs.muiline.com/view.php?id=24) - read/write/replace (see http://bugs.muiline.com/view.php?id=38)
Created attachment 103616 [details] [review] gvfs-obexftp-4.patch Done: - create directory - delete file/directory - get FS information TODO (in addition to implementing read/write/rename): - fixing deletion not checking for children (eek!) - change free space calculation algorithm: https://bugs.launchpad.net/gnome-vfs-obexftp/+bug/120544 - implement "device went away": https://bugs.launchpad.net/gnome-vfs-obexftp/+bug/120558 (we received a disconnected signal on the session, how do we commit suicide?) - test in nautilus
Created attachment 103651 [details] [review] gvfs-obexftp-5.patch Done: - fixing deletion not checking for children (eek!) - change free space calculation algorithm: TODO (in addition to implementing read/write/rename): - implement "device went away": https://bugs.launchpad.net/gnome-vfs-obexftp/+bug/120558 (we received a disconnected signal on the session, how do we commit suicide?) - test in nautilus Pretty much all I can do until bug 511802 is fixed, and a POSIX API is added to obex-data-server.
Marcel mentions that we should use GMarkup instead of expat in the parsers. Those are self-contained, so it would be easy to port them, and eventually move them to openobex itself.
(In reply to comment #6) > (In reply to comment #5) > > My efforts never really left the experimental stage and now I have zero time to > > work on it :/. What I did was the code to enumerate paired obex ftp devices > > through the bluez dbus api. I can probably dig that out and put somewhere. > > Probably not needed right now then, as we use bluetooth-applet's device > selection widget to choose the device to browse. However it would be nice to show paired bluetooth devices anyway. One way to do this is to create a volume monitor; another is just to add this to the existing hal volume monitor (there's a system D-Bus connection already; will most likely not accept other deps as this is loaded into each process using gio/gvfs volume monitoring.)
Created attachment 105909 [details] [review] gvfs-obexftp-6.patch Updated patch against current SVN.
Created attachment 105931 [details] [review] gvfs-obexftp-7.patch - Implement (very hackishly) do_open_for_read and do_read - Handle some errors - Handle disconnected signal (and exit straight away, as the device went away) Still need to implement do_close_for_read and clean up the 2 functions above.
This fails (it should complain about the file not being there): $ ./gvfs-copy -p 'obex://[00:12:D2:79:B7:33]/MMC/foo.jpg' file:///home/hadess/Projects/Cvs/gvfs/programs/test.jpg Error copying file obex://[00:12:D2:79:B7:33]/MMC/foo.jpg: The specified location is not mounted This works (as it should): $ obex://[00:12:D2:79:B7:33]/MMC/Image000.jpg file:///home/hadess/Projects/Cvs/gvfs/programs/test.jpg In the first case, I get the whole path in the mount: method call sender=:1.976 -> dest=org.gtk.vfs.Daemon path=/org/gtk/vfs/mounttracker; interface=org.gtk.vfs.MountTracker; member=lookupMount struct { array [ byte 47 byte 77 byte 77 byte 67 byte 47 byte 102 byte 111 byte 111 byte 46 byte 106 byte 112 byte 103 ] array [ struct { string "host" array [ byte 91 byte 48 byte 48 byte 58 byte 49 byte 50 byte 58 byte 68 byte 50 byte 58 byte 55 byte 57 byte 58 byte 66 byte 55 byte 58 byte 51 byte 51 byte 93 byte 47 byte 77 byte 77 byte 67 byte 47 byte 102 byte 111 byte 111 byte 46 byte 106 byte 112 byte 103 ] } struct { string "port" array [ byte 48 ] } struct { string "type" array [ byte 111 byte 98 byte 101 byte 120 ] } ] }
Also problems with the file size never getting asked on copies. So gvfs-copy says things like: "progress 593050/0"
Created attachment 105979 [details] [review] gvfs-obexftp-8.patch Implement close_read() Problem in comment 21 moved to bug 518835 Follow glib coding-style Handle user-cancellation in a lot of paths, including read() Should be ready to commit.
Some comments from a quick scan: if (same_paths (filename, current_path) == TRUE) passed in names are canonicalized, so isn't strcmp enough here? for the unimplemented vfuncs, just leave them not filled in You want more info about '/'. For instance display name (used as window title in nautilus) and icon. cancellation is weird. I don't understand how that works at all. Each op is separately cancellable. You can listen to the cancelled signal on the job if you want to catch that, or you can get an fd to select on from the GCancellable. When you get this the operations should be terminated and it should return the error G_IO_ERROR_CANCELLED. In general, the way you have to keep listing folders the backend seems quite inefficient. Does it not make sense to cache directory listings?
FWIW, the old gnomefs obex backend caches the latest used directory listing, and we were even thinking about making it cache even a bit more (like the full hierarchy from the current dir towards the root) since it takes quite a bit of time to keep retrieving that on some phones. The downside of course being that you might get an outdated listing.
Its slightly better in gvfs vs gnome-vfs, as the cache is shared between apps. So we can invalidate the cache as soon as any process does a write op. Also, cached results can be handled in e.g. try_query_info so these can finish even if the backend is busy doing other i/o.
(In reply to comment #23) > Some comments from a quick scan: > > if (same_paths (filename, current_path) == TRUE) > passed in names are canonicalized, so isn't strcmp enough here? Removed. > for the unimplemented vfuncs, just leave them not filled in Removed as well. > You want more info about '/'. For instance display name (used as window title > in nautilus) and icon. Done. > cancellation is weird. I don't understand how that works at all. > Each op is separately cancellable. You can listen to the cancelled signal on > the job if you want to catch that, or you can get an fd to select on from the > GCancellable. When you get this the operations should be terminated and it > should return the error G_IO_ERROR_CANCELLED. I got that completely wrong. I'll fix it. > In general, the way you have to keep listing folders the backend seems quite > inefficient. Does it not make sense to cache directory listings? That's supposed to be done in obex-data-server directly. http://bugs.muiline.com/view.php?id=35
Created attachment 106001 [details] [review] gvfs-obexftp-10.patch - Avoid races in do_open_for_read() by using condvars to signal that the open succeeded or failed - Remove same_paths usage - Remove unneeded - Make mount sync - Add a nice display name for the query on '/' (string already translated in the smb backend) - Use the "bluetooth" icon, better suited The cancellation still isn't fixed.
Created attachment 106010 [details] [review] gvfs-obexftp-11.patch - when cancelling a read, don't cancel the transfer, we might want to read again a bit later, only cancel transfers on close - correctly implement cancellation (hopefully).
That looks much better. I think the mutex/cond stuff is a bit racy though, and could be done better. For instance, the main thread can at any time get a callback to error_occurred_cb, right? And that checks op_backend->mutex != NULL and then does stuff. But the mutex is destroyed in another thread without mutual exclusion, so it may go away after the check has returned true. A better approach would be to allocate mutex and cond once, at initialization. Then have a separate doing_io boolean you check in the callbacks. This boolean should only be set and checked inside the mutex lock. This means that any backend operation should just grab the mutex at the start and then release it at the end. When actually sleeping on the cond waiting for a wakeup the mutex will be automatically releases by g_cond_wait() and then reaquired on return, so you'll not hold the mutex for a long time anyway (and thus not risking to have the main thread sleep on the mutex long). Also, shouldn't cancelled_cb set op_backend->error?
Created attachment 106060 [details] [review] gvfs-obexftp-12.patch - Fix locking following Alex' comments cancelled_cb should only be called when we cancel the transfer from do_close_read(), so it's not an error. (Note that this can also happen if the remote device cancels one download, but I can't reproduce that problem here).
Fixed the ->data use, there's not point in it being a pointer. We're missing write support (moved to bug 519071). 2008-02-27 Bastien Nocera <hadess@hadess.net> * client/test-uri-utils.c: add test for obex URIs * configure.ac: Require expat for the obexftp backend * daemon/Makefile.am: * daemon/gvfsbackendobexftp-cap-parser.c: * daemon/gvfsbackendobexftp-cap-parser.h: * daemon/gvfsbackendobexftp-fl-parser.c: * daemon/gvfsbackendobexftp-fl-parser.h: Added ObexFTP folder listing and capability parser from gnome-vfs-obexftp, ported to gio * daemon/gvfsbackendobexftp.c: * daemon/gvfsbackendobexftp.h: * daemon/obexftp-marshal.list: * daemon/obexftp.mount.in: Add read-only ObexFTP backend (Closes: #509621)
(In reply to comment #9) > - Doesn't allow listing obex:// (I think somebody else will have to work on > that, as it's not visible in the UI, we use bluez-gnome to list devices) Wouldn't it be much nicer to have a bluetooth "subfolder" in network:// which would link to obex://? bluetooth-applet would just handle certain notifications and give access to the bluetooth preferences. Sending files is better done using nautilus-sendto and bluetooth file-sharing is better done using gnome-user-share.
(In reply to comment #32) > (In reply to comment #9) > > - Doesn't allow listing obex:// (I think somebody else will have to work on > > that, as it's not visible in the UI, we use bluez-gnome to list devices) > > Wouldn't it be much nicer to have a bluetooth "subfolder" in network:// which > would link to obex://? bluetooth-applet would just handle certain notifications > and give access to the bluetooth preferences. Sending files is better done > using nautilus-sendto and bluetooth file-sharing is better done using > gnome-user-share. Sending files uses bluetooth-sendto directly, file sharing uses gnome-user-share. But the point of entry using bluez-gnome is better than presenting the device through nautilus. You're welcome to disagree, just file another bug, I'm not interested in working on it.
Once an obexftp mount is mounted it will be visible in computer: and other places in the ui anyway. This is not browsing really as you have to first find and mount it via somewhere else, but it does mean its easy to find it. However, it would be nice to have an obex specific volume monitor so that paired phones are easy to find for mounting.