Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FYI: Sync Calibre to KOReader seems broken, user feedback needed #28

Closed
kyxap opened this issue Jul 27, 2024 · 21 comments
Closed

FYI: Sync Calibre to KOReader seems broken, user feedback needed #28

kyxap opened this issue Jul 27, 2024 · 21 comments
Assignees
@kyxap

Comments

@kyxap
Copy link
Collaborator

kyxap commented Jul 27, 2024 •
edited
Loading

Hi all,

image

The feature is broken at this moment and I'm not sure if I there anyone who is using it. So please let me know if it's needed! And I will try to fix it as well.

Current issue on windows:

  1. If running in non debug mode:
calibre, version 7.15.0
ERROR: Unhandled exception: <b>PermissionError</b>:[WinError 5] Access is denied: 'Utopia of Rules_ On Technology, Stupidity, and the Secret Joys of Bureaucracy, The - David Graeber &amp; Devid Griebier.sdr'

calibre 7.15  embedded-python: True
Windows-10-10.0.22631-SP0 Windows ('64bit', 'WindowsPE')
('Windows', '10', '10.0.22631')
Python 3.11.5
Windows: ('10', '10.0.22631', 'SP0', 'Multiprocessor Free')
Interface language: None
EXE path: C:\Program Files\Calibre2\calibre.exe
Successfully initialized third party plugins: Extract ISBN (1, 6, 3) && Goodreads (1, 8, 2) && Goodreads Sync (1, 16, 8) && KOReader Sync (0, 6, 2)
Traceback (most recent call last):
  File "calibre_plugins.koreader.action", line 674, in sync_missing_sidecars_to_koreader
  File "calibre_plugins.koreader.action", line 610, in push_metadata_to_koreader_sidecar
  File "os.py", line 225, in makedirs
PermissionError: [WinError 5] Access is denied: 'Utopia of Rules_ On Technology, Stupidity, and the Secret Joys of Bureaucracy, The - David Graeber & Devid Griebier.sdr'
  1. If running calibre in debug (e.g. terminal pwd is in Downloads)
image

Sidecars will be dowaloded locally in that folder:

But UI going to show that all sidecar is missing
image

calibre, version 7.15.0
Success: 6 books on device without sidecars.
Sidecar creation succeeded for 6.
Sidecar creation failed for 0.
No attempt made for 0 (no metadata in Calibre to push).
See below for details.

  {
    "result": "success",
    "book_id": 311,
    "book_uuid": "d840bffc-c9b9-4043-8424-e4c2a4ff8854",
    "sidecar_path": "Utopiia pravil. O tiekhnologhiiakh, ghluposti i tainom obaianii biurokratii - David Graeber & Devid Griebier.sdr/metadata.epub.lua"
  }
]

Some logic bug and some file access problem as well.

Put your 👍 or 👎 if you need this or better to focus on something else
@kyxap kyxap added question Further information is requested user feedback needed Something that can be used to get user feedback to do upvote/downvote for feature requests, etc labels Jul 27, 2024
@kyxap kyxap self-assigned this Jul 27, 2024
@kyxap kyxap pinned this issue Jul 27, 2024
@kyxap kyxap changed the title FYI: Sync Calibre to KOReader is broken and disabled as of now v0.6.3 FYI: Sync Calibre to KOReader is broken and disabled as of v0.6.3 Jul 27, 2024
@kyxap kyxap added question Further information is requested and removed question Further information is requested labels Jul 27, 2024
@charlesangus
Copy link
Collaborator

charlesangus commented Jul 27, 2024 via email

@charlesangus
Copy link
Collaborator

charlesangus commented Jul 27, 2024 via email

@kyxap
Copy link
Collaborator Author

kyxap commented Jul 27, 2024

It's a useful feature when you need it, which is basically just when setting up a new reader, or after a factory reset/similar wipe off your reader. Not really any other way to get eg your highlights/bookmarks back on to the reader that I'm aware of.

I see your point but usually it's much simpler to backup (and recomended) koreader or just books with sidecar then rely on external software :)

@kyxap
Copy link
Collaborator Author

kyxap commented Jul 27, 2024

But from that traceback, it sounds like you have a permissions issue, which may or may not have much to do with the plugin itself?

yeah, I assume because I'm using windows but in debug mode it still did not work (file was places in windows folder instead of device) so still need to debug and find the root cause.

@charlesangus
Copy link
Collaborator

charlesangus commented Jul 28, 2024 via email

@kyxap
Copy link
Collaborator Author

kyxap commented Jul 31, 2024

Put your voice then.
Also do you wanna test it on your end and see how it works with all recent changes? I can upload version with enabled feature for testing

@charlesangus
Copy link
Collaborator

I wrote this feature initially, so yeah, I can look into why it's not working for you if/when I can find the time.

Things I can see from looking over the code and your errors:

  • may not be getting the path correctly for this case (which is not a real device, I suspect it gets the path ok for a real device)
  • some kind of permissions issue (possibly related - looks like it may be trying to write to pwd/your_path or /your_path instead of the fully-qualified path, which would potentially do it
  • bad error handling (should be reporting failure, not success)

Doesn't look hard to fix.

kyxap added a commit that referenced this issue Aug 2, 2024
@kyxap
Enabling sync to koreader with exception cetch for Permission issue to
1849273 
help with #28
kyxap added a commit that referenced this issue Aug 2, 2024
@kyxap
Enabling sync to koreader with exception catch for Permission issue to
3c86c38 
help with #28
@kyxap kyxap mentioned this issue Aug 2, 2024
Merged
kyxap added a commit that referenced this issue Aug 5, 2024
@kyxap
Merge pull request #32 from harmtemolder/sync-to-koreader-enabled-wit…
aac66dc 
…h-extra-exception-catch

Enabling sync to koreader with exception catch for Permission issue to help with #28
@kyxap kyxap changed the title FYI: Sync Calibre to KOReader is broken and disabled as of v0.6.3 FYI: Sync Calibre to KOReader seems broken Aug 5, 2024
@kyxap kyxap changed the title FYI: Sync Calibre to KOReader seems broken FYI: Sync Calibre to KOReader seems broken, user feedback needed Aug 5, 2024
@kyxap
Copy link
Collaborator Author

kyxap commented Aug 5, 2024

enabled in v0.6.5 to get user feedback

@onatbas
Copy link

onatbas commented Aug 22, 2024 •
edited
Loading

Thank you I've been looking for this functionality. I'll give this a go and test and let you know my findings.

Edit: As others have mentioned. I think this is particularly useful when you have a new device or you've moved around the books and lost the metadata. If you don't have the metadata already synced into calibre, it isn't actually creating the metadata.

image

I would've loved if it could sync into the metadata of the books (create the files if needed) things such as series information, reading status (in-progress, unread, read etc).

From a sync to koreader functionality, the solution I would be most happy about is if the plugin let me move books around different folders and still detect them. As koreader users will now, organization is a pain in the ***.

@kyxap
Copy link
Collaborator Author

kyxap commented Aug 22, 2024

@onatbas are on linux? I'm not sure if I understand does it work for you or not? If not how this different from reported issue in the first post?

from the use case that you mentioned, if you getting new device I think it pretty simple to backup your books with metadata/sidecar and then just copy over to new device.

I've looked into the code got much more question then answers, so at this point does not look like easy fix at least for me :)

@onatbas
Copy link

onatbas commented Aug 22, 2024

I'm on Windows. I think for the intended purpose of the feature so far, it works. But I was expecting other things from it as it is called "Sync to Koreader". I didn't realize in the end that this is a Metadata Backup/Restore feature.

I have moved to KOReader brand new from a non-koreader based reader (kobo nickel). And I have no metadata to begin with, but I do have categorization of read,unread books etc. So I was hoping this sync operation would help me by creating all the metadata for the books I already synced to my ereader but I will not be opening them for a good long while (because I read them). They're in the read folder, but KOreader doesn't know that.

@WillScarlettOhara
Copy link

WillScarlettOhara commented Aug 26, 2024 •
edited
Loading

309406
For the first time since I started using KOReader, it worked for me as I wrote on mobileread.
Windows 11
Calibre 7.17.0
Plugin v0.6.7
KOReader 2024.07-85
Kobo Libra 2

@kyxap
Copy link
Collaborator Author

kyxap commented Aug 26, 2024

@WillScarlettOhara seems like you are using wired connection which actually may work some how... (1 book got sidecar, yay)

Anyway, msg that you have 1295 books without side car means you need get back all those sidecars from calibre to koreader and you got only for 1. I assume your book are in calibre

@kyxap
Copy link
Collaborator Author

kyxap commented Aug 26, 2024

Sync via wifi after adding extra try catch #32
v0.6.7
image

@WillScarlettOhara
Copy link

@WillScarlettOhara seems like you are using wired connection which actually may work some how... (1 book got sidecar, yay)

Anyway, msg that you have 1295 books without side car means you need get back all those sidecars from calibre to koreader and you got only for 1. I assume your book are in calibre

Wired connection yes. Again, it didn't work at all for me before (several months ago, I'd say).
It was only a test. I deliberately reset a book on KOReader. I don't want to overwrite the KOReader sidecars because perhaps my reading progress is more advanced on KOReader than it is on Calibre.

@TheOnlyWayUp
Copy link
Contributor

Small fyi, it errors on crash logs (cvm/core files) in the docs dir. No complaints after I deleted em

@harmtemolder
Copy link
Owner

harmtemolder commented Sep 16, 2024 •
edited
Loading

I've noticed another error when using this feature:

the sidecar in calibre

vs.

the restored sidecar on the device

As you can see, it only takes the last digit of the highlight numbering, resulting in duplicate keys in the Lua.

Edit: Looks like something is being done explicitly with this numbering here:

koreader-calibre-plugin/action.py

Line 615 in 07ef544

sidecar_lua = re.sub(r'\["([0-9])+"\]', r'[\1]', sidecar_lua)

Edit 2: Commenting out that line fixed my issue. I'll create a pull request for that. Other than that, this functionality worked like a charm on my Linux machine

@harmtemolder harmtemolder mentioned this issue Sep 16, 2024
Merged
@GreyFoxRTX
Copy link

309406 For the first time since I started using KOReader, it worked for me as I wrote on mobileread. Windows 11 Calibre 7.17.0 Plugin v0.6.7 KOReader 2024.07-85 Kobo Libra 2

I am having this exact issue. It will push NOTHING to KOReader.

@wfdewith
Copy link

There is a fundamental problem with the current architecture. The Lua objects in the sidecar files are parsed and then encoded as JSON, and this operation is not lossless. Lua supports different types of object keys, while JSON only supports strings.

To give an example of where this behavior causes problems. Given some Lua object as follows.

{
  [5] = "value"
}

This object will be stored as the following JSON object in Calibre.

{
  "5": "value"
}

However, the following Lua object leads to exactly the same JSON object in Calibre.

{
  ["5"] = "value"
}

When restoring the JSON object back to a Lua object to put in the sidecar file, you can either choose to encode the key to [5] or ["5"]. Before #52, the behavior was to encode JSON keys like "<integer>" to [<integer>] in Lua, but now they are always encoded to ["<integer>"] in Lua. Both options are ultimately wrong! However, the behavior before #52 was less problematic, since KOReader doesn't appear to use ["<integer>"] keys in Lua, so the incorrect encoding was never encountered.

The specific issue in #28 (comment) is caused by an incorrect regex; it should be \["([0-9]+)"\] instead of \["([0-9])+"\].

I can make a PR reverting #52 and fixing the regex such that we're less likely to run into incorrect restoring behavior. However, I'm not sure if it is sensible to assume that KOreader will never create keys formatted like ["<integer>"]. There is no clean way to fix the ambiguity as long as JSON is being used as an intermediate format. This raises the question of why even use JSON as intermediate format (see also #58). Can we just keep the Lua source without any processing in a column in the Calibre library? Do we want to still expose JSON (in another column) for people who want to write scripts consuming this data (for example, with calibredb)? I'm curious to hear your thoughts @kyxap.

@kyxap
Copy link
Collaborator Author

kyxap commented Dec 10, 2024

not sure why json was used, maybe because calibre config using json for configuration and it's more "popular" data format to operate.

@kyxap
Copy link
Collaborator Author

kyxap commented Dec 10, 2024

Anyway it seems like Sync Calibre to KOReader actually "works" for most of the people. So I'm going to close this thread and if you have specific bug/issue please create new one. Thanks

@kyxap kyxap closed this as completed Dec 10, 2024
@kyxap kyxap removed question Further information is requested user feedback needed Something that can be used to get user feedback to do upvote/downvote for feature requests, etc labels Dec 10, 2024
@kyxap kyxap unpinned this issue Dec 10, 2024
@wfdewith wfdewith mentioned this issue Dec 10, 2024
Open
@harmtemolder harmtemolder mentioned this issue Dec 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kyxap kyxap

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@onatbas @wfdewith @kyxap @charlesangus @harmtemolder @WillScarlettOhara @TheOnlyWayUp @GreyFoxRTX