Skip to content

Kobo v5 support#11

Merged
NiLuJe merged 4 commits into
NiLuJe:masterfrom
NerdyProjects:kobo_v5
Aug 25, 2024
Merged

Kobo v5 support#11
NiLuJe merged 4 commits into
NiLuJe:masterfrom
NerdyProjects:kobo_v5

Conversation

@NerdyProjects

@NerdyProjects NerdyProjects commented Aug 24, 2024

Copy link
Copy Markdown
Contributor

This adds a change to support Kobo v5 thumbnails, it also adds an init script to start kfmon on boot.

It still misses the tooling to generate an update.tar as there might be some more testing neccessary anyway.

ToDos

  • See if the new thumbnail detection is enough or triggers false positives
  • think about proper special character conversion for new thumbnail filenames - as it is only for shortcuts that don't need crazy special char support, it's likely fine as it is. Current conversion list is from a few files I had on my ereader...
  • generate update.tar

This change is Reviewable

Comment thread kfmon.c
}

if (thumbnails_count == 0U) {
char converted_book_path[CFG_SZ_MAX + 7];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KFMON_PATH_MAX (or, I guess sizeof(thumbnail_path), even)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I followed the definition of book_path which gets copied into this.

@NiLuJe NiLuJe Aug 25, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which was a sensible decision ^^.

Or I guess another approach would just be to replace in-place, since it's only going to be 1:1 replacements?

Ooh, which raises a new question: does Nickel also do a byte-for-byte replacement, or does it do a per-codepoint replacement for multibyte glyphs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file:///mnt/onboard/Books/The Will to Change_ Men, Masculinity, and Love -- bell hoo►ks -- 20$04 -- A§tria Booöküs -- 0743480333 -- fa╚8.epub

becomes

file____mnt_onboard_Books_The_Will_to_Change__Men,_Masculinity,_and_Love_--_bell_hoo►ks_--_20$04_--_A§tria_Booöküs_--_0743480333_--_fa╚8_epub

The file system mounts with CP437 for me, afaik there is no multi byte characters in a fat filename, so that should not be an issue. I'll check your list of characters again, but it seems everything else is just fine.

Comment thread kfmon.c
is_processed = true;
}

if (thumbnails_count == 0U) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably make the whole thing branch on whether we're actually running on v5 or not to avoid mixing this with the v3/v4 codepath ;).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point but I don't see a downside on the mixed approach, as it seems we don't need to clearly identify a v5 (yet). Otherwise, would do that earlier in Code...

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. For future reference, does /mnt/onboard/.kobo/version still exist?

@NerdyProjects NerdyProjects Aug 25, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

root@ereader:~# cat /mnt/onboard/.kobo/version 
T302NNNNNNNNN,4.9.77,5.2.190625,4.9.77,4.9.77,00000000-0000-0000-0000-000000000691

(queried via ssh :D )

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks ;).

Comment thread kfmon.c Outdated
{
for (size_t i = 0; i < length; i++) {
unsigned char c = str[i];
if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c == ',') || (c == '-') || (c == '&') || (c == '?') || (c == 0) || (c >= '0' && c <= '9')) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems different than the list of illegal FAT32 chars. Was that from empirical evidence or just something to quickly start with?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sure helps when I actually read your first post, lol. Never mind me ;p.

@NiLuJe NiLuJe Aug 24, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, said list is NUL " * / : < > ? \ | (according to https://en.wikipedia.org/wiki/Comparison_of_file_systems)

@NerdyProjects NerdyProjects marked this pull request as ready for review August 25, 2024 11:45
@NerdyProjects

NerdyProjects commented Aug 25, 2024

Copy link
Copy Markdown
Contributor Author

I added a new kobov5 Makefile target to generate the update zip files for install and uninstall.
You have to extract them onto the base of the usb storage.

I also replaced pkill by killall - I hope that's not a problem? As I kept the filename of the uninstaller script kfmon-uninstall.sh it got killed during the uninstall :D

@NerdyProjects

Copy link
Copy Markdown
Contributor Author

KFMon-.zip
KFMon-Uninstaller.zip

Here are my builds of this branch, is anybody wants to try out :-)

@NiLuJe

NiLuJe commented Aug 25, 2024

Copy link
Copy Markdown
Owner

I also replaced pkill by killall - I hope that's not a problem? As I kept the filename of the uninstaller script kfmon-uninstall.sh it got killed during the uninstall :D

Oh, good point ;p.

Nah, should be fine, we only really wanted to kill the actual daemon there.

@NiLuJe NiLuJe merged commit 9c3043c into NiLuJe:master Aug 25, 2024
@NiLuJe

NiLuJe commented Aug 25, 2024

Copy link
Copy Markdown
Owner

Thanks!

(I may tweak a few things, so I might ping you to double-check I didn't break anything ;)).

@NiLuJe

NiLuJe commented Aug 25, 2024

Copy link
Copy Markdown
Owner

There, pushed a few things, hopefully I didn't break anything ^^.

@NerdyProjects

Copy link
Copy Markdown
Contributor Author

I updated to your master branch - it's working fine! thanks for tidying up - I lack a bit of practice there...

@NiLuJe

NiLuJe commented Aug 25, 2024

Copy link
Copy Markdown
Owner

No worries, thanks for being willing to work on this and test it in the first place ;).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants