[Device Plugin] Kobo Touch Extended Driver

[JSWolf]

Originally Posted by theducks

@meeera Your code is probably very clean
This is probably caused by a minor code error in one of the files.

FWIW Very rarely (1 time every couple of months. been happening for a couple of years), I send an EPUB as KEPUB and the file is corrupt when I go to read it on my Kobo. I must delete the book then resend . Using the Validate in the Editor, does not see any issues.
(Win X64 Calibre)

You also need to use epubcheck and have both say the ePub is clean.

[chaley]

Originally Posted by davidfor

This is probably going to be a pain to work out. The only thing I can think of is that a separate thread is being started to do the work for each file. Something is happening that either one is failing or getting "lost". There have been some issues on Macs with the thread management with some occasional crashes. This could be a less obvious symptom of that.

When it happens, there might be an error in the calibre debug log. But, to see it, you would need to run calibre in debug mode and check the log. Or keep it until you see a book with the problem. Either way it will be painful.

1. I don't think this will help, but one thing you might try is to tell the thread pool to use fewer threads. By default it uses 'number_of_processors * 5'. You can reduce that by giving "max_workers=X" (for some X) in "with ThreadPoolExecutor() as pool" in container.py.__run_async().
2. Another thought. __run_async() waits for a maximum of 10 seconds for each threaded task submitted to the pool. It waits in task submission order. Even if any task running by itself is guaranteed to finish in 10 seconds, it is very possible that a task early in the queue is starved by other tasks and violates the timeout. This can also happen if the machine is busy. This raises two questions:
   • Why 10 seconds? Why not unlimited (None)? Why not 100 seconds?
   • It isn't obvious what happens if finish() raises TimeoutError, which it will do when the timeout is hit.
     • It seems that finish() won't be called for the remaining tasks, leaving their threads waiting to terminate.
     • It seems that the exception will eventually be caught and logged, but what processing is skipped is hard (for me) to see.
3. In any event, after looking at the docs I don't think the code is right. The python docs suggests a different structure for the code. It wants the calls to finish() to be inside the "with" statement, which makes sense because the thread pool will be cancelled when the "with ..." statement completes. It also suggests that the finish() call be wrapped in a try/except block.
   
   This doc is also good, but long and somewhat hard to read. It suggests that your case should look something like the Submit and Use as Completed pattern. Here is an example. I left the 10 second timeout, but as before I am not sure why it is there. A TimeoutError is raised if 10 seconds pass with no task finishing.
   

       from concurrent.futures import as_completed
       from concurrent.futures import TimeoutError
       with ThreadPoolExecutor() as pool:
           futures = [pool.submit(func, *arg) for arg in args]
           try:
               for future in as_completed(futures, timeout=10):
                   try:
                       future.result()
                   except Exception as e:
                       print(f'Exception {str(e)} waiting for task to finish')
           except TimeoutError as t:
               print('A task took too long')

[davidfor]

Looking at all that, I suspect that reducing the number of threads in the pool and increasing the timeout would do it. But, the documentation shows that the defaults for the number of threads changed in Python 3.8 to "min(32, os.cpu_count() + 4)". And I don't know why @jgoguen chose 10 seconds. I can see that an actual runtime of 10 seconds for any of these threads would mean a large file was being processed, but, a book with a lot of files could get some timing out as you say.

Looking at the code, I agree that the loop over the futures should be inside the with. But, I'm not sure what that would help.

And I agree that the sample you have shown would be better. I'll have a look at it. I need to get the code in the beta out of the way first. Not that they interfere, just that I haven't gotten around to committing it and raising the pull request.

[CyberPaul]

Hi,
I'm newbie on Kobo. Is the latest version of KoboTouchExtender (3.5.4) compatible with the just released 4.32 firmware? Can I safely force using collections and books removal?
Thanks in advance

[ownedbycats]

Originally Posted by CyberPaul

Hi,
I'm newbie on Kobo. Is the latest version of KoboTouchExtender (3.5.4) compatible with the just released 4.32 firmware? Can I safely force using collections and books removal?
Thanks in advance

Yes. I've been using it.

[CyberPaul]

Thanks!

[davidfor]

Originally Posted by CyberPaul

Hi,
I'm newbie on Kobo. Is the latest version of KoboTouchExtender (3.5.4) compatible with the just released 4.32 firmware? Can I safely force using collections and books removal?

The short answer us yes.

The longer answer is that I will always post in the thread for a new firmware the status of the calibre and the firmware. I don't usually explicitly mention the extended driver, but, the extended driver is inheriting the function from the built-in driver, so there is no difference in they works.

[fogice]

Originally Posted by davidfor

This is probably going to be a pain to work out. The only thing I can think of is that a separate thread is being started to do the work for each file. Something is happening that either one is failing or getting "lost". There have been some issues on Macs with the thread management with some occasional crashes. This could be a less obvious symptom of that.

When it happens, there might be an error in the calibre debug log. But, to see it, you would need to run calibre in debug mode and check the log. Or keep it until you see a book with the problem. Either way it will be painful.

I am happy to try this, but it seems that subsequent comments say that it wouldn't help? Let me know what I should to to assist.

[davidfor]

Originally Posted by fogice

I am happy to try this, but it seems that subsequent comments say that it wouldn't help? Let me know what I should to to assist.

I would be interested in a debug log to see if anything shows, but, it doesn't happen all the time, so we would have to be lucky to catch it. And I think @chaley's analysis is correct. I just have to find the time to test it.

[chaley]

Originally Posted by davidfor

Looking at the code, I agree that the loop over the futures should be inside the with. But, I'm not sure what that would help.

The threading code is not easy to understand!

That said, when the ThreadPoolExecutor instance is garbage collected, which can happen any time after the "with" exits, some of the internal structure for the pool goes away. This is detected by the threads using weakrefs. It seems that in this case a thread can terminate instead of doing the next task on the queue. If this is true, and if there are more tasks than threads (more files than threads), then sometimes a task will be skipped. Putting the future.result() calls inside the "with" avoids this possibility.

I won't stake my reputation on this analysis.

[davidfor]

Originally Posted by chaley

The threading code is not easy to understand!

Just noted in the quoted text I had "what that" instead of "that that" though it would have been better to us "that it".

That said, when the ThreadPoolExecutor instance is garbage collected, which can happen any time after the "with" exits, some of the internal structure for the pool goes away. This is detected by the threads using weakrefs. It seems that in this case a thread can terminate instead of doing the next task on the queue. If this is true, and if there are more tasks than threads (more files than threads), then sometimes a task will be skipped. Putting the future.result() calls inside the "with" avoids this possibility.

I won't stake my reputation on this analysis.

Yep, that would explain it. Garbage collection and weakrefs are a pain. I did a lot of SmallTalk development at one time. Someone had created a SQL database class that used weakrefs as a caching method and was supposed to trigger a read to the database if the data wasn't there. But, the garbage collector could do the cleanup between the check that the data was there and the attempt to actually get the data. The bug was a obvious when I found it, but, a real pain to find. And surprisingly easy to fix.

Anyway, I had to fix something else and posted a beta in another thread (forgot where I was). So, I have added this change and attached the beta here. This has:

• Use template for whether to kepubify - see this for details.
• Reorganisation of the options. Again, see this for details.
• Fix for renaming sending kepubs in library - see this for details.
• Change in threading.

The version number will be 3.5.7. And I really need to check those earlier changes in.

[fogice]

Originally Posted by davidfor

So, I have added this change and attached the beta here.

Excellent! I will delete everything from my Libra 2, turn on debug mode, and reload everything using this beta to see if the missing chapters are fixed. Should I wait until
(a) the next Calibre release (you said something about book length would be included, but I didn't completely follow) and/or
(b) update from 4.30 to 4.32 (the reports look good, but I usually prefer to wait a week)?

[davidfor]

Originally Posted by fogice

Excellent! I will delete everything from my Libra 2, turn on debug mode, and reload everything using this beta to see if the missing chapters are fixed. Should I wait until
(a) the next Calibre release (you said something about book length would be included, but I didn't completely follow) and/or
(b) update from 4.30 to 4.32 (the reports look good, but I usually prefer to wait a week)?

There is no need to wait. This is independent of the firmware on the device. When I do this sort of thing, what I do is take a full copy of the books partition. If something goes wrong, or when I have finished testing, I can restore it and not lose reading statistics or anything else.

Firmware 4.32 displays some extra statistics for books from Kobo. They are the page and word count, plus an estimate of reading time. These are the same as shown in the store. When you update the firmware, any books from Kobo will get these populated at the next sync. I have added updating these values in the built-in driver. That will be in the next calibre release and the extended driver will inherit this behaviour. If you want to use it, you will need to do some configuration.

[Terisa de morgan]

Originally Posted by davidfor

Firmware 4.32 displays some extra statistics for books from Kobo. They are the page and word count, plus an estimate of reading time. These are the same as shown in the store. When you update the firmware, any books from Kobo will get these populated at the next sync. I have added updating these values in the built-in driver. That will be in the next calibre release and the extended driver will inherit this behaviour. If you want to use it, you will need to do some configuration.

Just out of curiosity, which values will be used for updating these fields? Custom columns? So, a nice relationship with CountPages?

[ownedbycats]

Originally Posted by Terisa de morgan

Just out of curiosity, which values will be used for updating these fields? Custom columns? So, a nice relationship with CountPages?

That's what I would assume, yes. Count Pages plugin can generate the page and wordcounts and put them in columns.

Interestingly, I was considering dropping the word count column as I found it less useful than page and chapter counts. But now I have a reason to keep it.