Koreader progress sync interface #3025

MFDeAngelo · 2024-06-28T00:34:22Z

Added

Added: Added native support to sync progress with Koreader. See this issue on the Koreader repository for more information.

MFDeAngelo · 2024-06-28T00:34:39Z

This PR is not ready for a merge (if that isn't already apparent.) I wanted to show you the basic interface that I'd like to implement and get some feedback on it before diving in. Here are some relevant details that will help you understand the scope of the proposed change.

As it currently stands, you can read books on Koreader by using OPDS and stream the book from Kavita. Unfortunately, I have two issues with this. First, I haven't even gotten it to work yet. I've only tried a handful of times, but it always produces just the Koreader logo. It might simply be user error, but if you like, I can investigate further and open an issue if necessary. Second, what if I'm away from my LAN and want to sync my books? What if the power goes out? Well too bad. This feature should solve that issue.

Koreader has a plugin that ships with the install called 'progress sync'. It is on by default. It allows a user to connect to a Koreader sync server, to sync progress across Koreader devices. We can use this functionality within Koreader to sync progress between Koreader devices and Kavita. A user would only need to us OPDS to download the book onto their Koreader device, then authenticate to the server in the progress sync page, and poof! The Koreader device and Kavita can sync progress!

The user will have to enter the following url into their Koreader device: 'http://my-kavita-server.com/api/koreader/api-key/'. Then they will login using their username, and the password can be anything, as the authentication happens through the api-key. (I don't think it makes sense to use the user's actual password, as Koreader sends the MD5 hash of the password to the server. Since Kavita doesn't store passwords as an MD5, it would make implementing this much more difficult.) Alternatively we could require the user to use the api-key as the password. Your advice would be appreciated.

Koreader allows you to register as a user. I think it's in our best interest to leave that functionality out.

There are Put and Get endpoints that allow for syncing between the device and the server. Koreader uses the MD5 hash of the book (or filename as a secondary option) to identify what book is being synced. I'm not sure how to handle the device id stuff yet, but that seems like an implementation concern.

Let me know what you think. I'd love to implement this functionality.
Also, I'd like to thank you for putting together this wonderful piece of software. I've really enjoyed using it. It was the second thing I installed on my home server, and I'm glad I did!

majora2007 · 2024-06-28T12:32:57Z

First off, I would welcome a PR for this (and any needed PR for OPDS and Koreader).

(I don't think it makes sense to use the user's actual password, as Koreader sends the MD5 hash of the password to the server. Since Kavita doesn't store passwords as an MD5, it would make implementing this much more difficult.) Alternatively we could require the user to use the api-key as the password. Your advice would be appreciated.

I agree. I don't want to use passwords with external applications, hence why we have the api key in the first place.

Koreader allows you to register as a user. I think it's in our best interest to leave that functionality out.

Agreed. External user provisioning is not something I'm a fan of for Kavita.

My main points of concern are this:

Make sure you use Dto classes when appropriate with good comments
Try to write some unit tests on the actual sync (unless it's as simple as calling SaveProgress())
Let's make sure the code is separated into a Koreader controller (which you've already done) and any Koreader service if needed.
Please have in mind that documentation will be needed for the wiki as well once this is implemented.

Let me know if you need anything either via Github or via discord.

MFDeAngelo · 2024-06-30T22:32:24Z

Wonderful! I'll be working on this over the next couple of weeks. I've looked through code, and I think I have a general idea of how to accomplish it. Thanks for your help!

MFDeAngelo · 2024-07-11T23:12:51Z

Not sure why this says it was closed by me. I had no intention of closing it yet.
Just wanted to give an update saying that I'm still working on it. I've been able to establish a connection from koreader to kavita, and now am working on converting the progress datatypes from one to another.

majora2007 · 2024-07-12T00:39:29Z

Awesome. I think AutoMapper can help with that as much as possible. It's okay if the PR gets closed, as long as you re-open once you're ready for some more review.

MFDeAngelo · 2024-08-20T00:06:43Z

@majora2007 I've made some more changes and have it working locally. When reading in koreader, the progress is updated on kavita, and vice versa. However, I don't feel confident that I've accounted for every potential "bookScrollId". I wanted to know what you thought about releasing this feature in beta to start off with, with the expectation that it might break here and there.

I'll get this branch caught up on the develop branch in the next couple of days, improve the logging, and start preparing the Wiki entry. Could you perform a quick once over to give me any feedback on it?

majora2007 · 2024-08-20T00:50:54Z

Yeah, ill go over it and give you a review. I think we can probably get this into the current nightly, since we still have some good amount of time before the next stable goes out.

majora2007

Lots of style comments and the need for documentation to get the full gist.

I def would recommend adding some links to places where you learned to reverse engineer, which will not only help me, but any other project that wants to add this sort of support.

API/API.csproj

API/Controllers/KoreaderController.cs

API/Services/KoreaderService.cs

majora2007 · 2024-08-21T12:46:56Z

API/Services/KoreaderService.cs

+            Device_id = "kavita",
+            Device = "kavita",
+            Progress = koreaderProgress,
+            Percentage = 0.5f


Curious why we are sending a percentage if it's not correct...

By what I've seen, Koreader doesn't appear to use this value. I think it could be used by the Koreader progress sync server to display statistics or something. I'll see if I can track it down a little better.

So I've been looking at this even more, and it appears to be only for message boxes.
A little earlier in the method it compares against a previous percentage and a new percentage to see if if is in the same position. If so, it isn't supposed to update the progress in Koreader. However, in testing it doesn't behave that way. Maybe it has to do with direct comparison of a floating point.

It looks like it's a user friendly percentage. Can we not use the Progress mapped to a 0-1 float?

Someone from Stump did some research on this:
stumpapp/stump#239 (comment)

majora2007 · 2024-08-21T12:47:43Z

API/Services/Tasks/Scanner/ProcessSeries.cs

@@ -818,6 +818,7 @@ private void AddOrUpdateFileForChapter(Chapter chapter, ParserInfo info, bool fo
            var file = new MangaFileBuilder(info.FullFilePath, info.Format, _readingItemService.GetNumberOfPages(info.FullFilePath, info.Format))
                .WithExtension(fileInfo.Extension)
                .WithBytes(fileInfo.Length)
+                .WithHash()


I'd love to know the overhead of hashing files, perhaps a benchmark can help us understand. (See the Benchmark project)

I set up the benchmark and will be committing it soon. I set up the benchmark to build a MangaFile using the MangaFileBuilder. I went ahead and ran the benchmark on hashing a 300 KB epub. Here are the results:

| Method | Mean | Error | StdDev | Median | Ratio | RatioSD | Rank | Allocated | Alloc Ratio | |------------------------ |----------:|----------:|----------:|----------:|------:|--------:|-----:|----------:|------------:| | TestBuildManga_baseline | 40.18 us | 1.631 us | 4.731 us | 38.49 us | 1.00 | 0.00 | 1 | 922 B | 1.00 | | TestBuildManga_withHash | 230.09 us | 10.654 us | 31.415 us | 229.86 us | 5.81 | 0.99 | 2 | 7151 B | 7.76 |

One thing to keep in mind about the hashing algorithm is that a 300KB file will only have 4KB of its contents hashed. At most, the algorithm will hash 10 KB of the file. That means that even though there are files much larger than 300KB, they shouldn't take that much longer to hash.

For Koreader, is this only for epub files? Because if so, I think we could swallow this type of overhead. If it's for archives as well, then we might want to look into offloading this into a separate task that performs the hashing. This will offload the extra spend on the scan loop and shortly make it available after scan for syncing.

I switched it to only hash EPUBs. I don't think there is any sense in hashing the other files as I don't believe Koreader supports Archives. It supports PDFs though. We can add support for that later.

API/Services/Tasks/Scanner/ProcessSeries.cs

API/Data/Migrations/20240704154945_BookHash.cs

MFDeAngelo · 2024-09-14T00:24:15Z

@majora2007, I've merged the most recent changes into my branch and updated the PR. I've made a few small changes since the last time I ran the project. however, I noticed that I can't run the project anymore. I launch the API project and it directs me to localhost, but it returns an HTTP 500.30. I even pulled straight from the Kavita develop branch, and am getting the same result. I'm not sure if it is my local setup, or if there is something wrong in develop.

Regardless, I'd like to run a few quick tests, then I'm ready for an in depth review. If we can resolve the HTTP 500 error, then I'll run those tests and get it to you to review.

majora2007 · 2024-09-14T11:38:17Z

I just pulled down your code and the webapp is working just fine, I'm not able to reproduce any 500 errors. Are you also launching the webui server as well with angular? You should be going to localhost:4200/

majora2007 · 2024-09-14T13:33:21Z

API/Entities/MangaFile.cs

@@ -21,6 +21,10 @@ public class MangaFile : IEntityDate
    /// </summary>
    public required string FilePath { get; set; }
    /// <summary>
+    /// A hash of the document using Koreader's unique hashing algorithm
+    /// </summary>
+    public string KoreaderHash { get; set; }


This should be string? as we aren't using for all file types. I would also update <remarks> to include information about the supported file types.

majora2007

I did a code review, I will pull this down soon and start testing the heck out of it.

@DieselTech if you want to also start testing.

We missed our opportunity for the v0.8.3 release, but v0.8.4 development is right around the corner and I know many people will want this feature.

majora2007 · 2024-09-14T13:33:39Z

API/Helpers/Builders/MangaFileBuilder.cs

+        {
+            _mangaFile.KoreaderHash = KoreaderHelper.HashContents(_mangaFile.FilePath);
+        }
+        return this;


Newlines above returns

majora2007 · 2024-09-14T13:33:58Z

API/Helpers/KoreaderHelper.cs

+    /// <param name="filePath">The path to the file to hash</param>
+    public static string HashContents(string filePath)
+    {
+


Remove empty line

majora2007 · 2024-09-14T13:34:33Z

API/Helpers/KoreaderHelper.cs

+
+        using var file = File.OpenRead(filePath);
+
+        int step = 1024;


Use var whenever possible

majora2007 · 2024-09-14T13:34:59Z

API/Helpers/KoreaderHelper.cs

+        var fileName = Path.GetFileName(filePath);
+        var fileNameBytes = Encoding.ASCII.GetBytes(fileName);
+        var bytes = MD5.HashData(fileNameBytes);
+        return BitConverter.ToString(bytes).Replace("-", string.Empty);


Newline above returns

majora2007 · 2024-09-14T13:37:02Z

API/Services/KoreaderService.cs

+            Device_id = "kavita",
+            Device = "kavita",
+            Progress = koreaderProgress,
+            Percentage = 0.5f


It looks like it's a user friendly percentage. Can we not use the Progress mapped to a 0-1 float?

MFDeAngelo · 2024-09-20T23:42:40Z

@majora2007 I fixed the stylistic concerns you had. I'm not sure how exactly to map a progress to a float though. It seems like it could be difficult, because of the way Kavita tracks a user's progress. I could use currentPage/totalPages, but that isn't the same way that Koreader tracks progress, and would lead to discrepancies. Do you have any other ideas?

majora2007 · 2024-09-21T13:30:11Z

@majora2007 I fixed the stylistic concerns you had. I'm not sure how exactly to map a progress to a float though. It seems like it could be difficult, because of the way Kavita tracks a user's progress. I could use currentPage/totalPages, but that isn't the same way that Koreader tracks progress, and would lead to discrepancies. Do you have any other ideas?

Well Kavita does technically use currentPage/totalPage. It just stores an additional xpath to resume scroll position, but the underlying progress would be currentPage/totalPage. This would ensure that the modal on Koreader side shows something correct.

I'll try and pull this down this weekend and give it a round of review. I don't have Koreader, but we can deploy this to a test instance and get some testers before we merge it into develop.

majora2007 · 2024-09-23T14:19:17Z

@DieselTech Can we get this deployed to the dev instance so we can ask users to test it? I don't want to merge into nightly yet without more validation testing from people with actual devices. We can lock the users down to dedicated testers.

majora2007 · 2024-10-04T14:21:01Z

Just to update you, I'm planning to put this in a canary and asking for some testing before I merge it into develop. Canary is currently being used for a large refactor of the scan loop, to ensure it's not breaking anything before moving the code into nightly/develop. Once that wraps, this is up next.

calilex · 2024-10-15T03:39:00Z

This feature is great, I’m looking forward to it very much.

majora2007 · 2024-10-23T11:38:26Z

Just an update, canary should be coming to an end today/tomorrow. It's been nearly a week of just working on rewriting the scanner but we are at a good merge point. Once that occurs, I will be creating a new canary with this for testing (which should be quick, I already have a user to perform the extended testing).

Thank you for your patience. With bigger PRs, it's critical to ensure things work well before merging in and taking ownership.

majora2007 · 2024-10-24T13:44:49Z

If you could resolve the conflicts then I'll get you merged into canary and start the testing.

Discussion for testing:
https://discord.com/channels/821879810934439936/1298773623825367170

MFDeAngelo · 2024-10-25T23:54:55Z

Hi @majora2007, apologies for the delay. I've never worked with synchronizing a fork. I finally understand what having multiple remotes is for now, so I think I'll be a little better at merging these things.

Anyways, I've brought the branch up to date with the feature/koreader-sync branch.

API/Entities/MangaFile.cs

API/Helpers/Builders/MangaFileBuilder.cs

…eader progress sync

majora2007 · 2024-10-26T11:30:09Z

I'm going to do some cleanup for you and once done, I encourage you to read over the upcoming PR into canary.

Some items I need from you:

Descriptions for the KoreaderController methods
Acknowledgement the epub included is not under copywrite

majora2007 · 2024-10-26T12:24:57Z

@MFDeAngelo A few more things that I'll need implementation help on. In the KoreaderService, there are a lot of cases of data coming back null that aren't being handled. I'm going to augment the code with TODOs and push it up. Please branch off my branch and get everything in order.

You wont have to deal with any merges anymore since the code is in my own branch. I just need help since I don't have Koreader setup.

You can see the TODOs on the PR:
#3311

MFDeAngelo closed this Jul 3, 2024

MFDeAngelo force-pushed the develop branch from 6eaded0 to 0fa7df9 Compare July 3, 2024 01:27

Basics structure of koreader support including db migration

506ea91

MFDeAngelo reopened this Jul 11, 2024

koreader base complete

467c403

Fixed koreader tests

1fc696d

MFDeAngelo mentioned this pull request Aug 21, 2024

Add Koreader Integration Page Kareadita/Wiki-Nextra#14

Open

majora2007 requested changes Aug 21, 2024

View reviewed changes

majora2007 added enhancement New feature or request db-migration This story needs a DB Migration labels Aug 21, 2024

majora2007 reviewed Aug 21, 2024

View reviewed changes

API/Data/Migrations/20240704154945_BookHash.cs Outdated Show resolved Hide resolved

Renamed Hash to KoreaderHash and added Koreader documentation

50cc1af

MFDeAngelo marked this pull request as ready for review September 14, 2024 00:24

MFDeAngelo and others added 3 commits September 13, 2024 23:04

MangaFiles can only have a Koreader hash if it is an EPUB.

5b3073a

Merge branch 'develop' into develop

9ffc916

Updated migrations for koreader hash

d64bcab

majora2007 reviewed Sep 14, 2024

View reviewed changes

Koreader stylistic changes

fc7f84f

majora2007 mentioned this pull request Oct 23, 2024

Transcoding Support + More Image Format Support (JPEGXL) #3250

Open

majora2007 changed the base branch from develop to canary October 24, 2024 13:41

majora2007 added 2 commits October 25, 2024 09:22

Lots of Bugfixes (Kareadita#3308)

fc269d3

Bump versions by dotnet-bump-version.

eacf2b9

majora2007 changed the base branch from canary to feature/koreader-sync October 25, 2024 16:25

majora2007 added 2 commits October 25, 2024 13:55

Update to Angular 18 (Kareadita#3309)

d563ebf

Bump versions by dotnet-bump-version.

841db59

majora2007 reviewed Oct 26, 2024

View reviewed changes

API/Entities/MangaFile.cs Show resolved Hide resolved

majora2007 reviewed Oct 26, 2024

View reviewed changes

API/Helpers/Builders/MangaFileBuilder.cs Show resolved Hide resolved

MFDeAngelo added 4 commits October 25, 2024 23:34

Merged before Koreader push

a10eac2

Merge changes from only including epubs in koreader update

c86135c

sync to koreader feature branch

8ee5be6

Add logging to koreader service and return Kavita's percentage to kor…

32878a4

…eader progress sync

majora2007 approved these changes Oct 26, 2024

View reviewed changes

majora2007 merged commit 180b49b into Kareadita:feature/koreader-sync Oct 26, 2024
1 check failed

Koreader progress sync interface #3025

Koreader progress sync interface #3025

Conversation

MFDeAngelo commented Jun 28, 2024 • edited by majora2007 Loading

Added

MFDeAngelo commented Jun 28, 2024

majora2007 commented Jun 28, 2024

MFDeAngelo commented Jun 30, 2024

MFDeAngelo commented Jul 11, 2024

majora2007 commented Jul 12, 2024

MFDeAngelo commented Aug 20, 2024 • edited Loading

majora2007 commented Aug 20, 2024

majora2007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MFDeAngelo commented Sep 14, 2024

majora2007 commented Sep 14, 2024

Choose a reason for hiding this comment

majora2007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MFDeAngelo commented Sep 20, 2024

majora2007 commented Sep 21, 2024

majora2007 commented Sep 23, 2024

majora2007 commented Oct 4, 2024

calilex commented Oct 15, 2024

majora2007 commented Oct 23, 2024

majora2007 commented Oct 24, 2024

MFDeAngelo commented Oct 25, 2024

majora2007 commented Oct 26, 2024

majora2007 commented Oct 26, 2024

MFDeAngelo commented Jun 28, 2024 •

edited by majora2007

Loading

MFDeAngelo commented Aug 20, 2024 •

edited

Loading