feat: Add Recipe poll interval configuration#2264
feat: Add Recipe poll interval configuration#2264faraoman wants to merge 5 commits intoferdium:developfrom
Conversation
Alphrag
left a comment
There was a problem hiding this comment.
Hey @faraoman thank you for the PR, this is quite a good idea. 👍
Since we are putting it in, I suggest we make it so that it is customizable directly in the interface per service, so that users can choose directly what the interval in polling that would suit them instead of being fixed by the recipe itself.
In order to do that, it suffices to modify the service form EditServiceForm and EditServiceScreen files in a suitable way, so that users can update with the value of their choosing.
This means that this could be sufficient on its own and would not require a change in RecipeWebview per se.
If you do wish to code a default value for a recipe I would in fact recommend creating a new configuration attribute that is set inside the package.json file of a recipe, and then deal with reading that value when a service is created. (I am afraid that setting it into the webview would override any user-set value when the service goes out of hibernation if not dealt with properly.)
This way we get both the advantage of being able to modify it easily on the user side, and the possibility of the person which sets up a recipe to give a default value.
What do you think? 🙂 You can at least make the changes I requested, and if you fancy adding a default value for each recipe, you can either add it to this PR or create a new one (as well as updating the documentation on the ferdium-recipes repo).
src/models/Service.ts
Outdated
|
|
||
| @observable lastHibernated: number | null = null; // timestamp | ||
|
|
||
| @observable pollDelay: number = 2000; // interval for Recipe Polling |
There was a problem hiding this comment.
Instead of hardcoding this value here, please add a default value for pollDelay into the DEFAULT_SERVICE_SETTINGS variable of the config.ts file, and call that value here.
src/stores/ServicesStore.ts
Outdated
| } | ||
| case 'set-loop-delay': { | ||
| debug('Received set-loop-delay request from', serviceId); | ||
| const defaultDelay = ms('2s'); |
There was a problem hiding this comment.
Same as above, call the default from the config file.
src/stores/ServicesStore.ts
Outdated
| let parsedData = ms(`${args[0]}`); | ||
| if (parsedData < defaultDelay) parsedData = defaultDelay; | ||
|
|
||
| service.pollDelay = parsedData; |
There was a problem hiding this comment.
This should probably be done with with an action to avoid MobX complaining about trying to change observable value without one (if you're not sure where this happens, track the places where setDialogTitle is used).
| loop(fn, interval) { | ||
| if (interval && typeof interval === 'string') { | ||
| ipcRenderer.sendToHost('set-loop-delay', interval); | ||
| } |
There was a problem hiding this comment.
If you really want to make a change happen from the recipe webview, I would prefer you to create a new function there like setPollDelay instead of modifying the loop one. However, I am not sure even this is worth it: see my general comment for the reason and discussion.
There was a problem hiding this comment.
Now that you've removed the listener set-loop-delay and implemented it in the recipe settings, you can revert this file back to what it was, removing your modifications.
|
Hi! @Alphrag, thanks for you observations, I appreciate that. I agree with you, to make this modification, so I will edit the code to insert this observations and after I will repropose a new merge request. Thanks! |
|
Perfect, I look forward to it. As long as you modify the code on your branch, the PR will update itself, so it won't be necessary to create a new one. So once you have done all the modifications in your branch (and rebased on those that are on the ferdium-app/develop one), simply ping me here and I'll review again. It is better to avoid closing and reopening new PRs for the same thing to keep the flow of reviews in the same place 😉 |
|
Hi @Alphrag ! |
Alphrag
left a comment
There was a problem hiding this comment.
Thanks. A few more changes which I have laid out in the comments, but mostly two things:
- revert your changes on
RecipeWebview, - add a field to the form in the service editing window so that the user can input the poll value they want and validate the consistency of that value (both
EditServiceForm.tsxandEditServiceScreen.tsxshould be modified). For the sake of the user, you should also add a small text next to it in the form to give a few details regarding the default value, the fact that it should be entered in the appropriate format/value (this is up to how you validate the value entered), and a warning that entering a small value will mean more this service will consume more resources.
Once this is done, we also need to make a corresponding PR on the https://github.com/ferdium/ferdium-recipes repository to update the docs and the package.js script in order to account for the addition of that new config option pollDelay. If you're not sure what I mean, or prefer me to do it, just let me know. It is a very quick one, so good for the HacktoberFest 😉
| ); | ||
|
|
||
| this.pollDelay = ifUndefined<number>( | ||
| recipe.pollDelay <= data.pollDelay ? data.pollDelay : recipe.pollDelay, |
There was a problem hiding this comment.
At the current state of things, this has no impact because you don't allow the user to modify the value. In order for it to do something you need to modify the files EditServiceForm.tsx and EditServiceScreen.tsx in order to add a field where the user can input the value (which will appear on the window that allows for adding/modifying a service). Make sure to validate the value entered appropriately.
As for this line explicitly: why would you prevent a user from entering a value smaller than what the recipe suggests? At the current stage, since no recipe has a given value, the polling time is the default 2sec, and users would like to poll more often. This would prevent the user to input a lower value when initially creating the service.
There was a problem hiding this comment.
Hi @Alphrag
I already made some changes (I would like to improve my knowledge of Ferdium) and this is a first result

In this case, can be an useful integration add a switch (Enable/Disable Poll Delay) and show this customization only if the user would like to personalize the delay of the poll
Also, than you have more experiences on this project, did you know some validation (already coded) for numeric input? I'm asking this, because I can't found nothing about. I would like to avoid duplicated code.
Thanks
There was a problem hiding this comment.
Great!
I leave the decision to you to either add a switch to enable/disable custom poll delay (where toggling off resets to the recipe's poll delay value), or to simply allow an empty value, which is the default and corresponds to the recipe's delay. We do the latter in other places of the code, for example, to handle the accentColor field on the general settings, and the important part is how we validate that field —which in that case required dynamic handling whereas here we won't need that (cf EditSettingsForm.tsx)—.
I honestly don't remember of having a specific function that validates numeric input (they would normally be located in the helpers folder) and since you simply need to check the value that is passed is an integer, a Number.isInteger check would probably suffice, don't you think?
| recipe.pollDelay <= data.pollDelay ? data.pollDelay : recipe.pollDelay, | ||
| this.pollDelay, | ||
| ); | ||
| // console.log('ctor-recipe', recipe); |
There was a problem hiding this comment.
Please remove comments used to test out things.
| loop(fn, interval) { | ||
| if (interval && typeof interval === 'string') { | ||
| ipcRenderer.sendToHost('set-loop-delay', interval); | ||
| } |
There was a problem hiding this comment.
Now that you've removed the listener set-loop-delay and implemented it in the recipe settings, you can revert this file back to what it was, removing your modifications.
Pre-flight Checklist
Please ensure you've completed all of the following.
Description of Change
Added the ability to change the interval which every recipe's poll function is triggered
Motivation and Context
On Ferdium there is an hardcoded poll interval (2s) and for some recipe are useful to give the ability to change this interval to respect and not abuse the api rate. In addition this configuration allow the reducing of the energy/resources used to run the recipe itself if the timing are correctly setted up respect of the default 2s. For compatibility if no time configuration are passed or if the time interval is less the 2s, the default value remain 2s. To configure the poll the formalism is based on the
msnpm packagesExamples:
Checklist
pnpm prepare-code)pnpm testpassesRelease Notes
none