[feat] Add FFI RTC interface by Frenzie · Pull Request #969 · koreader/koreader-base

Frenzie · 2019-09-08T16:27:00Z

This will allow for scheduling wakeups.

Prerequisite for koreader/koreader#5335.

This will allow for scheduling wakeups. Prerequisite for <koreader/koreader#5335>.

Frenzie · 2019-09-08T16:29:59Z

ffi/rtc.lua

+    wake.enabled = enabled and 1 or 0
+
+    local err
+    local rtc0 = C.open("/dev/rtc0", bor(C.O_RDONLY, C.O_NONBLOCK))


Whoops, I forgot to make these configurable. Tbh it may just be a waste of time to do so.

The rtc path, you mean?

Yeah, probably not worth it ;p.

ffi/rtc.lua

poire-z · 2019-09-08T17:27:15Z

ffi/rtc.lua

+
+local RTC = {
+    _wakeup_scheduled = false,
+    _wakeup_scheduled_ptm = nil,


ptm = ? (I guess it's a pointer to a "tm" :) but some comment might help getting into all that (I know nothing about that, so... discovering/learning)

Will do. Btw, it's clearer if you look at rtc_h.lua.

poire-z · 2019-09-08T17:32:55Z

ffi/rtc.lua

+    end
+
+    if re == 0 then
+        if enabled then


you go on doing things above even if the first one failed? expecting all the others will fail so you still get re==-1 in the end?
I would expect a return nil, re, err in each of the ifs

Good point.

Although technically the close doesn't necessarily mean it's not scheduled. (Not that the current code makes that distinction.)

poire-z · 2019-09-08T17:34:22Z

ffi/rtc.lua

+    wake.time.tm_yday = -1
+    wake.time.tm_isdst = -1
+
+    wake.enabled = enabled and 1 or 0


If you want to disable it, you can provide any time value, including 0? (= it doesn't matter?)

Yeah, setting it to a time in the past would effectively disable it, and I don't think I actually tried what happens when you combine disabled with a time in the future. It can probably be removed.

Also see http://man7.org/linux/man-pages/man4/rtc.4.html

poire-z · 2019-09-08T17:37:56Z

ffi/rtc.lua

+end
+
+--[[--
+Get RTC wakealarm from system.


What does that mean?
Is it an alternative/different alarm?
Or just to get a previously set alarm, by another process or a previous run of KOReader?
And do we need to clear any alarm set on exit/crash?

RTC stands for Real Time Clock. It's the physical crystal-based, battery-powered ticking clock responsible for a good part of the time-keeping (as far as "wall-clock", i.e., real word time) is concerned ;).

Thanks, I knew the thing :)
I was wondering about the notion of "system alarm" (the one we fetch from rtc0) vs "our alarm" (that we store as an attribute) here :) which I ended up understanding after reading the whole file.

I'll add some explanation in the function description (+ internal references).

poire-z · 2019-09-08T17:43:19Z

ffi/rtc.lua

+    if not (alarm_epoch == alarm_sys_epoch) then return end
+
+    -- If our stored alarm and the provided task alarm don't match,
+    -- we're not talking about the same task. This should never happen.


Unless 2 features/plugins set each an alarm. This will happen to the oldest plugin that did, no?
(if task_alarm_epoch should be the same used to set the alarm, and that the plugins should store so they can provide it to this function later?)

It could happen if a rogue plugin avoided the WakeupMgr:addTask/removeTask mechanism, in which case the check could prevent WakeupMgr from running its top task. (However, I haven't currently implemented that in front.)

Frenzie · 2019-09-09T10:39:40Z

I'll merge this now [edit: after fixing the luacheck issues] since this syscall stuff should be fairly stable, plus I've gone over several nights of sleep since first writing it. @NiLuJe also directly assisted with the implementation of the first iteration, from which it hasn't substantially changed.

Then if you could please direct your critical eye to koreader/koreader#5335 I'd be much obliged. :-)

koreader/koreader-base#971 Fixes <#5347 (comment)>. Also includes: * [feat] Add FFI RTC interface (<koreader/koreader-base#969>) * Bump thirdparty/djvulibre to current master (<koreader/koreader-base#970>)

Frenzie · 2019-09-10T18:59:11Z

ffi/rtc.lua

+function RTC:setWakeupAlarm(epoch, enabled)
+    enabled = (enabled ~= nil) and enabled or true
+
+    local ptm = C.gmtime(ffi.new("int[1]", epoch))


@NiLuJe This was my new simplified approach (without a whole t = time_t etc.) but apparently I accidentally made the module ARM-specific because on my computer it errors saying it should be uint_64. 🤦‍♂️

Anyway, not an issue for the initial Kobo implementation but I figured I'd leave a note. I'll (hopefully) look into recomplexifying it after the finishing touches on the frontend implementation. Which I expected to have finished already but I had to rush that MuPDF patch and stuff. :-P

NiLuJe · 2019-09-10T20:03:41Z

@Frenzie: That's because time_t is actually a long int (c.f., echo | gcc -E -xc -include 'time.h' - | grep time_t), so, 32bit on ARM, 64bit on x64 ;).

What you're doing here should work just fine with ffi.new("time_t[1]", epoch) instead.

Frenzie · 2019-09-10T20:11:31Z

@NiLuJe I think there might've been TZ complications. Either way, I won't look at it today.

NiLuJe · 2019-09-10T20:18:20Z

I don't see how that could be the case, there's no magic involved, time_t is just a typedef:

typedef long int __time_t;
typedef __time_t time_t;

So

local ptm = C.gmtime(ffi.new("time_t[1]", epoch))

is essentially doing

time_t foo = epoch;
struct tm* ptm = gmtime(&foo);

(Actual storage for what ptm points to is allocated as a static by the libc, i.e., subsequent calls will re-use it, hence the _r variant for thread-safety where it's the caller's job to pass a storage location, not that it's relevant here ;p).

Cf. <koreader#969 (comment)>. Otherwise the interface won't work on… platforms where it doesn't matter, but it's better to be correct. ;-)

Cf. <#969 (comment)>. Otherwise the interface won't work on… platforms where it doesn't matter, but it's better to be correct. ;-)

koreader/koreader-base#971 Fixes <koreader#5347 (comment)>. Also includes: * [feat] Add FFI RTC interface (<koreader/koreader-base#969>) * Bump thirdparty/djvulibre to current master (<koreader/koreader-base#970>)

Frenzie added the enhancement label Sep 8, 2019

[feat] Add FFI RTC interface

2a00f1d

This will allow for scheduling wakeups. Prerequisite for <koreader/koreader#5335>.

Frenzie force-pushed the rtc branch from 2f40cec to 2a00f1d Compare September 8, 2019 16:27

Frenzie commented Sep 8, 2019

View reviewed changes

Add some (unverified) documentation stub

be7fa00

poire-z reviewed Sep 8, 2019

View reviewed changes

ffi/rtc.lua Show resolved Hide resolved

poire-z reviewed Sep 8, 2019

View reviewed changes

Frenzie added 3 commits September 8, 2019 21:49

Add all the docs and comments

cb4d0cf

Address @poire-z comment

2c47d50

More accurate header comment

3df4409

fix undefined variable access

6ffc661

Frenzie merged commit 4c25c8f into koreader:master Sep 9, 2019

Frenzie deleted the rtc branch September 9, 2019 10:44

Frenzie mentioned this pull request Sep 9, 2019

Bump base for [fix] MuPDF Android font dir patch update koreader/koreader#5352

Merged

Frenzie commented Sep 10, 2019

View reviewed changes

Frenzie added a commit to Frenzie/koreader-base that referenced this pull request Sep 12, 2019

[fix] Use correct time_t type

353030e

Cf. <koreader#969 (comment)>. Otherwise the interface won't work on… platforms where it doesn't matter, but it's better to be correct. ;-)

Frenzie mentioned this pull request Sep 12, 2019

[fix] Use correct time_t type #975

Merged

Frenzie added a commit that referenced this pull request Sep 12, 2019

[fix] Use correct time_t type (#975)

4c40a45

Cf. <#969 (comment)>. Otherwise the interface won't work on… platforms where it doesn't matter, but it's better to be correct. ;-)

Frenzie mentioned this pull request Sep 18, 2019

Poweroff script addition koreader/koreader#3033

Closed

Conversation

Frenzie commented Sep 8, 2019

Uh oh!

Frenzie Sep 8, 2019 • 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!

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 8, 2019 • 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!

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

Uh oh!

Uh oh!

Frenzie Sep 10, 2019 • 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!

NiLuJe commented Sep 10, 2019

Uh oh!

Frenzie commented Sep 10, 2019

Uh oh!

NiLuJe commented Sep 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Frenzie Sep 8, 2019 •

edited

Loading

Frenzie Sep 8, 2019 •

edited

Loading

Frenzie commented Sep 9, 2019 •

edited

Loading

Frenzie Sep 10, 2019 •

edited

Loading

NiLuJe commented Sep 10, 2019 •

edited

Loading