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.

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

Add CronAPI #657

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add CronAPI #657

wants to merge 12 commits into from

Conversation

lvogt
Copy link

@lvogt lvogt commented Feb 4, 2024

This implements a cron-like API which uses AlarmManager and WorkManager

WorkManager is used to keep phone awake during task execution, and to be able to handle constraints - including stopping the task if constraint is no longer met.

Until now this has been only tested in the emulator, testing on my actual phone is next on my list. So consider this PR a "draft" but I wanted to get this out to get some "earlier" feedback about the whole concept.

Part of this relies on termux/termux-app#3821
PR for api-packages script: termux/termux-api-package#182

This implements a cron-like API which uses AlarmManager and WorkManager

WorkManager is used to keep phone awake during task execution, and to
be able to handle constraints - including stopping the task if
constraint is no longer met
@lvogt
Copy link
Author

lvogt commented Feb 5, 2024

I reread part of the AlarmManager documentation and made a few changes e584b5d and 06d680f

Any insights whether I got that right now are appreciated.

EDIT: Maybe switching to exact alarms as default is better approach anyway...

@agnostic-apollo
Copy link
Member

Thanks for implementing this. Looks good and clean overall, but its too big to review or merge at this time by me, lot of things would need to be looked at in regards to android alarms and your cron design, not going to be possible before the next app updates.

@lvogt
Copy link
Author

lvogt commented Feb 8, 2024

I understand, take your time.

}

return String.format(Locale.getDefault(),
"%s-%d-%s", executableUri.getLastPathSegment(), jobId, String.copyValueOf(randomId));
Copy link
Member

Choose a reason for hiding this comment

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

Use FileUtils.getFileBasename((UriUtils.getUriFilePathWithFragment(executableUri)) possibly with null check, read the java docs for why. However, I have locally added the com.termux.execute.executable extra for the path in TermuxService to get away from this uri mess.

Copy link
Author

@lvogt lvogt Feb 9, 2024

Choose a reason for hiding this comment

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

Did you actually encountered paths with # ? I know it's allowed, but it feels very wrong ;)

Anyway I applied your suggestion. Thanks.

Edit: To be honest: I should have written "I just tried it and its allowed (at least on ext4)" instead of "know" :D

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, some user reported it somewhere for termux-open, it got fixed with termux/termux-app@3e518a6

Linux allows all characters/bytes other than null byte, # isn't that unlikely, it's an ascii character after all.

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