06-11-2011, 05:57 PM | #46 |
Grand Sorcerer
Posts: 11,741
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
What matters is when control is returned to Qt for event processing. The sequence
done() start_next_job() and start_next_job() done() are equivalent because done won't actually be called until the GUI thread event handlers return to Qt. There may be some difference in the order of events in the queue, but I am not convinced that Qt preserves strict queue order. |
06-11-2011, 05:58 PM | #47 |
creator of calibre
Posts: 43,858
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
Doesn't look like it. device jobs are started in the DeviceManager thread, while update is called in the JobsManager thread, so there is no consistent semantics.
Maybe, I shall change DeviceJob to have the done func called in the device manager thread. |
Advert | |
|
06-11-2011, 05:59 PM | #48 |
Grand Sorcerer
Posts: 11,741
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
|
06-11-2011, 06:01 PM | #49 |
creator of calibre
Posts: 43,858
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
|
06-11-2011, 06:05 PM | #50 |
Grand Sorcerer
Posts: 11,741
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
My suspicion is that you are starting down a path toward rewriting device jobs.
|
Advert | |
|
06-11-2011, 06:07 PM | #51 |
creator of calibre
Posts: 43,858
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
|
06-11-2011, 06:20 PM | #52 |
creator of calibre
Posts: 43,858
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
Turned out to be a fairly simple patch, committed. Tested with a couple of devices.
@chaley: Please have a look 2meme: You should be good to go. |
06-11-2011, 06:28 PM | #53 |
Grand Sorcerer
Posts: 11,741
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
@kovid: is DeviceJob.job_done called synchronously before JobManager starts the next job? (Yes, I am being lazy, but that code is ultra-mysterious. )
|
06-11-2011, 06:29 PM | #54 |
creator of calibre
Posts: 43,858
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
Yeah it is called in DeviceManager.run() line 263 which call run() on the DeviceJob which in turn calls job_done()
|
06-11-2011, 06:40 PM | #55 |
Grand Sorcerer
Posts: 11,741
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
That change makes sense, assuming I understand what is happening (a dangerous assumption). It calls 'done' on the same thread that runs the job, which is the right thing to do, and perhaps makes the is_gui_thread changes in FunctionDispatcher irrelevant. It clearly does not dequeue the next job until done.
Regardless of whether or not this fixes meme's problem, it is a good change, clarifying the semantics of DeviceJob termination. |
06-11-2011, 07:57 PM | #56 |
creator of calibre
Posts: 43,858
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
Yeah its certainly a much clearer call chain Lets hope we haven't missed anything critical. The only thing that gives me a little worry is the aborting of a device job, when for example a device is yanked while many jobs are queued. I tried it and nothing terrible happened, but given the amount of concurrence, a single test is meaningless.
FD is used elsewhere as well, so I think I'll leave the is_gui_thread check in there, since it makes sense. |
06-12-2011, 07:15 AM | #57 |
Sigil developer
Posts: 1,275
Karma: 1101600
Join Date: Jan 2011
Location: UK
Device: Kindle PW, K4 NT, K3, Kobo Touch
|
Ok - the good news is this seems to fix the serialization of sending books to the device.
I can use Reading List to send new books to the Kindle, and I can now see that these books are listed as on_device in calibre when Create runs. Send Metadata runs after Create (it seems to get queued at the end of sync_to_device, but clearly the on_device status doesn't depend on send metadata which is good). Thank you for all the investigation/updates to get this to work! But now I think I have a bit of work to do on my code this week... One problem is that I am displaying a dialog in my job_completed function that is causing calibre to crash. The issue is that within the job I instantiate a class (for messages) using the gui passed into the job - and I return that instantiated class and try to call its display method. calibre promptly warns me about pixmaps in a non-gui thread, missing parents, etc. and dies. So I need to re-arrange this code to only return the message strings to the job's parent and use the gui in the job's parent to present the dialog. Ah, if I had only coded it properly to begin with (Interesting that this didn't fall over with 0.8.5, just with the modified source) Another problem, which I'm not sure if its mine or not, is that I if I run my plugin while the Kindle is not connected, I get: Code:
Traceback (most recent call last): File "threading.py", line 530, in __bootstrap_inner File "/home/meme/Calibre-Source/calibre/src/calibre/gui2/device.py", line 262, in run self.device.set_progress_reporter(job.report_progress) AttributeError: 'NoneType' object has no attribute 'set_progress_reporter' I guess I can add a check to see if the device is connected before I kick off the job - but it seems to me it should be okay to do this and the job should queue until the device is connected? This behaviour is the same in 0.8.5. |
06-12-2011, 08:41 AM | #58 | |||
Grand Sorcerer
Posts: 11,741
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
Quote:
Quote:
The big difference in the new code is that the done functions are now called on the job thread, not the GUI thread. For this reason it is *required* that they be wrapped with Dispatcher() or FunctionDispatcher(), so that the thread switch can happen. Quote:
|
|||
06-12-2011, 10:23 AM | #59 |
creator of calibre
Posts: 43,858
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
I committed code to automatically wrap the callback in a FD if it isn't already wrapped and to not error out when someone tries to run a device job when no device is connected.
|
06-12-2011, 10:52 AM | #60 |
Sigil developer
Posts: 1,275
Karma: 1101600
Join Date: Jan 2011
Location: UK
Device: Kindle PW, K4 NT, K3, Kobo Touch
|
Downloaded new source. Seems to work for both issues.
When the plugin runs and returns, it now displays the dialog/messages ok, without me having to make any changes to my code. If the Kindle is not connected when the plugin runs, calibre no longer errors and my plugin issues the usual warning about no device. So that's what Dispatcher/FunctionDispatcher are for - thanks for wrapping it automatically so I don't need to add it myself I'll do more testing, but it looks great! One thing I did notice is that my job is not fully complete until the user confirms OK in the info/error dialog I present in my complete function (the job is marked 100% but not Finished), so Send Metadata doesn't run until I press OK. But all I need to do is present the report to the user - the function is fully complete and I don't really want to hold up any queued jobs waiting for the report to be read. It doesn't matter (and might not be a good idea), but it might be nice to avoid the block - is there a different way to call one of the dialog functions? |
|
Similar Threads | ||||
Thread | Thread Starter | Forum | Replies | Last Post |
eBooks from the library —waiting waiting | Khendron | Kobo Reader | 9 | 05-12-2010 01:02 PM |
Which one of these activities do you dedicate more time to? | daviddem | Lounge | 24 | 12-26-2008 11:40 AM |
iRex to expand B2B activities / speaks of possible IPO | Alexander Turcic | iRex | 12 | 04-10-2007 09:36 AM |