Skip to content

Added rescan interval #4

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.

Sign up for GitHub

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

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

Added rescan interval #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ba0ff17
Added rescan interval
MystaraTheGreat Mar 15, 2021
8c5222b
Fixed error in which function became out of scope
MystaraTheGreat Mar 15, 2021
a376de8
Added documentation
MystaraTheGreat Mar 17, 2021
6642845
Changed tab to spaces
MystaraTheGreat Mar 21, 2021
dbc00cd
Changed documentation to mention that scanInterval is in seconds
MystaraTheGreat Mar 21, 2021
2f309a4
Removed package-lock.json
MystaraTheGreat Mar 21, 2021
a4d1e57
Changed tabs to spaces
MystaraTheGreat Mar 21, 2021
fa3e05a
Rescan interval
MystaraTheGreat Apr 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion MMM-RandomPhoto.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Module.register("MMM-RandomPhoto",{
opacity: 0.3,
animationSpeed: 500,
updateInterval: 60,
scanInterval: -1, // How often to look for new photos. -1 = never
imageRepository: "picsum", // Select the image repository source. One of "picsum" (default / fallback), "localdirectory" or "nextcloud" (currently broken because of CORS bug in nextcloud)
repositoryConfig: {
// if imageRepository = "picsum" -> "path", "username" and "password" are ignored and can be left empty
Expand Down Expand Up @@ -41,6 +42,7 @@ Module.register("MMM-RandomPhoto",{

start: function() {
this.updateTimer = null;
this.scanTimer = null;
this.imageList = null; // Used for nextcloud and localdirectory image list
this.currentImageIndex = -1; // Used for nextcloud and localdirectory image list
this.running = false;
Expand Down Expand Up @@ -70,8 +72,19 @@ Module.register("MMM-RandomPhoto",{
},

fetchImageList: function() {
var self = this;

Log.info(this.name + " fetching image list");
if (typeof this.config.repositoryConfig.path !== "undefined" && this.config.repositoryConfig.path !== null) {
this.sendSocketNotification('FETCH_IMAGE_LIST');

if (this.config.scanInterval != -1) {
Copy link
Owner

@skuethe skuethe Mar 23, 2021

Choose a reason for hiding this comment

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

The problem with this timer is, that it will conflict with the updateInterval timer.
Also, when you have startPaused set to true it will still reload the images and continue with the image cycle on every scanInterval trigger. This breaks usage of the startPaused setting and somewhat the one of updateInterval.

So we need to think of a way to respect these settings before triggering a reload.

Copy link
Author

@MystaraTheGreat MystaraTheGreat Mar 23, 2021

Choose a reason for hiding this comment

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

  1. Conflict

Could you please explain this in more detail? I'm not really sure I see how there's a conflict. The two timers should be independent.

  1. startPaused set to true

I think the issue is that this.load() is getting called on line 107 when startPaused is set to true? Consequently even though startPaused is set to true, the image will keep cycling.

So to fix that, the IMAGE_LIST notification needs to be disconnected from loading an image - at least after an image has already been loaded?

Copy link
Owner

@skuethe skuethe Mar 23, 2021

Choose a reason for hiding this comment

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

  1. Conflict

Well this also has to do with the fact that we are calling this.load() on every IMAGE_LIST notification received. In this case both timers are triggering the load of the next image. That's what I mean with "conflict".

  1. startPaused set to true

Well this.load() is always called in the resumeImageLoading function. Regardless of what startPaused is set to.
startHidden actually handles the call when receiving the notification.

After giving it a bit of thought, it is probably best to just get rid of lines 411 to 413.

We are calling the resumeImageLoading function already during MM²'s extended build-in resume function. So that should get the image loading started in the first place.

But this needs to be tested to see if the app behaves like it should with all the different config settings - especially startHidden and startPaused.

Copy link
Author

@MystaraTheGreat MystaraTheGreat Mar 24, 2021

Choose a reason for hiding this comment

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

Okay. It does look as if they would solve the initial issue. But I agree that it's necessary to check the startHidden and startPaused. I'll give it a go here and see what happens.

Copy link
Author

@MystaraTheGreat MystaraTheGreat Mar 24, 2021

Choose a reason for hiding this comment

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

Hm, no, doesn't seem to work. The image loading never starts.
A messier fix might be to add a firstImageLoaded flag to indicate whether a first image has loaded or not - and not to call resumeImageLoading if firstImageLoaded is true.

But it seems inelegant.

Copy link
Owner

@skuethe skuethe Mar 24, 2021

Choose a reason for hiding this comment

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

If we delete lines 411 to 413 we need to also adjust these lines.
But the problem actually still persists. No display is being shown, because the image loading just takes more time.
So actually invoking the image showing on the IMAGE_LIST socketNotification still makes sense (lines 411 to 413).

So I would agree the easiest solution probably is to introduce some kind of flag / parameter to indicate if it is an "initial image loading" process or an "update image loading" process.

Some thoughts without going deeper into details:

  • maybe add an additional UPDATE_IMAGE_LIST socketNotification?
  • somehow wait for this.imageList's lenght to be > 0?
  • maybe work with async and await to wait for some executions?

Copy link
Author

@MystaraTheGreat MystaraTheGreat Mar 28, 2021

Choose a reason for hiding this comment

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

I'm now testing a version that uses an UPDATE_IMAGE_LIST socket notification. On reception, all it does it replace the imageList. It doesn't do anything regarding resumption.

I haven't followed up on waiting for the imageList length to be >0 or using async/wait. These changes don't seem to be necessary?

Copy link
Author

@MystaraTheGreat MystaraTheGreat Apr 3, 2021

Choose a reason for hiding this comment

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

The above version seems to work. However, I've been having difficulty with the calendar not updating as well, so I'm going to take the simpler option of just restarting MM every day.

Feel free to use the push or not as you like :)

clearTimeout(this.scanTimer);
this.scanTimer = setTimeout(function() {
Log.log("Requesting new images");
self.fetchImageList();
}, (this.config.scanInterval * 1000));
}
} else {
Log.error("[" + this.name + "] Trying to use 'nextcloud' or 'localdirectory' but did not specify any 'config.repositoryConfig.path'.");
}
Expand Down Expand Up @@ -106,7 +119,7 @@ Module.register("MMM-RandomPhoto",{
if (self.localdirectory || self.nextcloud) {
if (self.imageList && self.imageList.length > 0) {
url = "/" + this.name + "/images/" + this.returnImageFromList(mode);

jQuery.ajax({
method: "GET",
url: url,
Expand Down Expand Up @@ -399,6 +412,9 @@ Module.register("MMM-RandomPhoto",{
if(!this.config.startHidden) {
this.resumeImageLoading(true);
}
} else if (notification === "UPDATE_IMAGE_LIST") {
Log.log("Updating image list");
this.imageList = payload;
}
},

Expand Down
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ The entry in `config.js` can include the following options:
| `opacity` | *Optional* - The opacity of the image.<br><br>**Type:** `double`<br>**Default:** `0.3`
| `animationSpeed` | *Optional* - How long the fade out and fade in of photos should take.<br><br>**Type:** `int`<br>**Default:** `500`
| `updateInterval` | *Optional* - How long before getting a new image.<br><br>**Type:** `int`<br>**Default:** `60` seconds
| `scanInterval` | *Optional* - How long, in seconds, before rescanning the image repository. A value of -1 means 'never'.<br><br>**Type:** `int`<br>**Default:** `-1` (never)
| `startHidden` | *Optional* - Should the module start hidden? Useful if you use it as a "screensaver"<br><br>**Type:** `boolean`<br>**Default:** `false`
| `startPaused` | *Optional* - Should the module start in "paused" (automatic image loading will be paused) mode?<br><br>**Type:** `boolean`<br>**Default:** `false`
| `showStatusIcon` | *Optional* - Do you want to see the current status of automatic image loading ("play" / "paused" mode)?<br><br>**Type:** `boolean`<br>**Default:** `true`
Expand Down
20 changes: 17 additions & 3 deletions node_helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ const fs = require("fs"); // for localdirectory

const NodeHelper = require("node_helper");

var initialLoadCompleted = false;

module.exports = NodeHelper.create({

start: function() {
Expand Down Expand Up @@ -60,7 +62,13 @@ module.exports = NodeHelper.create({
}
}
this.imageList = imageList;
self.sendSocketNotification("IMAGE_LIST", imageList);
if (initialLoadCompleted == false) {
self.sendSocketNotification("IMAGE_LIST", imageList);
initialLoadCompleted = true;
} else {
self.sendSocketNotification("UPDATE_IMAGE_LIST", imageList);
}

return;
}
return false;
Expand Down Expand Up @@ -93,7 +101,13 @@ module.exports = NodeHelper.create({
imageList[index] = item.replace("href>" + urlParts.pathname, "");
//console.log("[" + self.name + "] Found entry: " + imageList[index]);
});
self.sendSocketNotification("IMAGE_LIST", imageList);
if (initialLoadCompleted == false) {
self.sendSocketNotification("IMAGE_LIST", imageList);
initialLoadCompleted = true;
} else {
console.log("Updating image list")
self.sendSocketNotification("UPDATE_IMAGE_LIST", imageList);
}
return;
} else {
console.log("[" + this.name + "] WARNING: did not get any images from nextcloud url");
Expand Down Expand Up @@ -158,4 +172,4 @@ module.exports = NodeHelper.create({
},


});
});