Exporter: XMNote support exporting read time and mark book source by Merpyzf · Pull Request #14144 · koreader/koreader

Merpyzf · 2025-08-05T14:32:19Z

XMNote users have long requested the ability to export daily reading time records from KOReader. This PR adds support for exporting reading time within the existing XMNote exporter plugin.

Additionally, to help XMNote App better organize imported books, each exported book is now marked with its source as “KOReader”.

This change is

…"KOReader"

…ng durations

Merpyzf · 2025-09-14T09:08:51Z

Hi, sorry to bother you. Perhaps pazos is busy and hasn’t had time to review my code. Would it be possible to assign this review to someone else? Thanks! @Frenzie

Frenzie · 2025-09-14T12:41:28Z

I'm not really sure if anybody else knows the Exporter plugin nearly as well but I'll take a quick look.

Frenzie · 2025-09-14T12:54:11Z

+function XMNoteExporter:getBookReadingDurations(id_book)
+    local conn = SQ3.open(db_location)


I see two presumably fairly minor issues with this:

It assumes the DB exists, but that's predicated on the statistics plugin being enabled. You'd have to include something like if not self.ui.statistics then return end.

This seems to look sufficiently generic that most of it should probably be located somewhere else, like ReaderStatistics itself or Exporter base. I'd go for ReaderStatistics itself unless there's a good reason not to.

Pinging @poire-z regarding ReaderStatistics.

The purpose of this query is to align with the field requirements defined by the export App’s API, which makes it fairly specific to the export logic. From a modular perspective, I agree that reading statistics generally belongs in ReaderStatistics. However, since this query is strongly coupled to the API field definitions, I decided to keep it in xmnote.lua to avoid introducing API-specific logic into a generic module.

Would you agree with this reasoning, or do you think it would still be better to move the core query into ReaderStatistics and leave only the API adaptation in xmnote.lua?

No strong opinion.
It indeed feels strange to have in this "remote" module some SQL query with internal knowledge about how the stats db is set up.
But I'm fine with not having in the statistics plugin tons of functions with SQL queries that are not used in it or by core :)

I wondered what this SQL query returns. I see that it gives results like:

dates last_page durations min_start_time ---------- --------- --------- -------------- 2025-09-10 9 78 1757538978 2025-08-31 11 14 1756656993 2025-08-30 11 20 1756560788 2025-08-28 11 46 1756367532 2025-08-25 12 126 1756125006 2025-08-09 14 417 1754694878

So, dunno. If put in the stats plugin, it should have a more explicite name, maybe getBookReadingDurationsByDay(id_book).

Mainly I was thinking that if this is useful for one export implementation maybe it also is for others. For example for Markdown/HTML the output from the same query could be written as a table at the bottom. But for now let's keep it in here, just with that early return right at the start.

If multiple export implementations need something from the stats db, may be it could just be an other specific module inside the exporter plugin (with a not in the statistics plugin that there are additional SQL queries in the exporter plugin).

I did write "or Exporter base." ;-)

No strong opinion. It indeed feels strange to have in this "remote" module some SQL query with internal knowledge about how the stats db is set up. But I'm fine with not having in the statistics plugin tons of functions with SQL queries that are not used in it or by core :)

I wondered what this SQL query returns. I see that it gives results like:

dates last_page durations min_start_time ---------- --------- --------- -------------- 2025-09-10 9 78 1757538978 2025-08-31 11 14 1756656993 2025-08-30 11 20 1756560788 2025-08-28 11 46 1756367532 2025-08-25 12 126 1756125006 2025-08-09 14 417 1754694878

So, dunno. If put in the stats plugin, it should have a more explicite name, maybe getBookReadingDurationsByDay(id_book).

I’m querying a book’s daily reading stats across all dates, capturing each day’s first reading time, the last page read, and the total reading duration. I’ll also adjust the SQL aliases to make them clearer and easier to understand.

Frenzie · 2025-09-14T12:54:35Z

+        SELECT
+            id
+        FROM
+            book
+        WHERE
+            title = "%s"
+        LIMIT 1


Not important but for such a simple query this seems a bit silly. :-)

I’ll change it to a single line. Thanks!

hius07 · 2025-09-14T14:52:02Z

+    return reading_durations
+end
+
+function XMNoteExporter:getBookIdByTitle(title)


Rare case, but book title may be not unique.

koreader/plugins/statistics.koplugin/main.lua

Lines 718 to 727 in 105694e

function ReaderStatistics:getIdBookDB()

local conn = SQ3.open(db_location)

local id_book

local sql_stmt = [[

SELECT count(id)

FROM book

WHERE title = ?

AND authors = ?

AND md5 = ?;

]]

If needed we can fetch the md5 from doc_settings along with title and author

koreader/plugins/exporter.koplugin/clip.lua

Lines 347 to 352 in af50d71

clippings[title] = {

file = doc_path,

title = title,

author = author,

number_of_pages = doc_settings:readSetting("doc_pages"),

}

I’ll work on optimizing it.

…e exporter

Frenzie · 2025-10-11T18:18:11Z

+local util = require("util")
+local ffiUtil = require("ffi/util")
+local datetime = require("datetime")
+local T = ffiUtil.template


Could you sort the new additions as per the coding style? https://github.com/koreader/koreader-base/wiki/Coding-style#order-of-requires

datetime ffiUtil logger util _ T

Sorry, I haven’t paid attention to that document before. If I had known, I would have done it. I’ll make the changes now!

Merpyzf · 2025-10-12T13:43:22Z

Hello, the CI check for this PR did not pass. After reviewing the logs, the error doesn't appear to be related to my code changes, which only involved adjusting the order of require statements. Would it be possible to manually re-trigger the pipeline? Thank you for your time.

hius07 · 2025-10-13T05:44:42Z

 end

+function XMNoteExporter:getBookReadingDurationsByDay(id_book)
+    if util.fileExists(db_location) then


No need to check again, already done in findBookIdByTitleAndMD5().

Frenzie

@hius07 Anything else?

hius07 · 2025-10-14T15:09:04Z

Everything looks correct, just a small note: do we need a separate findBookIdByTitleAndMD5()?
Can it be just a part of getBookReadingDurationsByDay()?
Currently the db is opened and closed twice.

Of course, if there are plans to get other info from the db in the future, it's okay.

Merpyzf · 2025-10-15T13:41:52Z

In this case, there’s no need to split it.

…educe db operations

…reader#14144)

Exporter: XMNote support exporting read time and mark book source as …

48bbcd5

…"KOReader"

Frenzie mentioned this pull request Aug 5, 2025

Exporter: support exporting read time and mark book source as "KOReader" #14143

Closed

Frenzie added the Plugin label Aug 5, 2025

Frenzie requested a review from pazos August 5, 2025 14:50

Fix xmnote plugin crash by checking nil book_id before fetching readi…

72005db

…ng durations

Frenzie reviewed Sep 14, 2025

View reviewed changes

Frenzie added this to the 2025.10 milestone Sep 14, 2025

hius07 reviewed Sep 14, 2025

View reviewed changes

Exporter: optimize and add support for reading status import in XMNot…

38294a2

…e exporter

Frenzie reviewed Oct 11, 2025

View reviewed changes

Exporter: reorder require statements in XMNote exporter

c062a04

hius07 reviewed Oct 13, 2025

View reviewed changes

Exporter: remove redundant if check in XMNote exporter

c7fe1ed

Frenzie approved these changes Oct 14, 2025

View reviewed changes

Exporter: merge book id lookup into getBookReadingDurationsByDay to r…

3663632

…educe db operations

hius07 approved these changes Oct 15, 2025

View reviewed changes

Frenzie merged commit 158463f into koreader:master Oct 15, 2025
4 checks passed

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026

Exporter: XMNote support exporting read time and mark book source (ko…

e75eb92

…reader#14144)

		function XMNoteExporter:getBookReadingDurations(id_book)
		local conn = SQ3.open(db_location)

	function ReaderStatistics:getIdBookDB()
	local conn = SQ3.open(db_location)
	local id_book
	local sql_stmt = [[
	SELECT count(id)
	FROM book
	WHERE title = ?
	AND authors = ?
	AND md5 = ?;
	]]

	clippings[title] = {
	file = doc_path,
	title = title,
	author = author,
	number_of_pages = doc_settings:readSetting("doc_pages"),
	}

Uh oh!

Uh oh!

Conversation

Merpyzf commented Aug 5, 2025 • edited by Frenzie Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Merpyzf commented Sep 14, 2025

Uh oh!

Frenzie commented Sep 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Frenzie Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Merpyzf commented Oct 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Frenzie left a comment

Choose a reason for hiding this comment

Uh oh!

hius07 commented Oct 14, 2025

Uh oh!

Merpyzf commented Oct 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Merpyzf commented Aug 5, 2025 •

edited by Frenzie

Loading

Frenzie Sep 14, 2025 •

edited

Loading