Cache and restore bridge IP addresses into global settings #30
Conversation
| // Save IP addresses for bridges into global settings | ||
| this.saveCachedBridges = function(cache) { | ||
| Object.keys(cache).forEach(function(id) { | ||
| globalSettings.bridges[id].ip = cache[id].ip; |
There was a problem hiding this comment.
This will override the cached ip.
- The discovery process might find a new ip for the same id (e.g. DHCP use-case) and we'll want to update the cache with the new one
| @@ -87,6 +87,9 @@ function connectElgatoStreamDeckSocket(inPort, inUUID, inRegisterEvent, inInfo, | |||
| else if (event === 'sendToPropertyInspector') { | |||
| // Save global cache | |||
| cache = jsonPayload; | |||
|
|
|||
| // Save Cached Bridges | |||
| pi.saveCachedBridges(cache); | |||
There was a problem hiding this comment.
I don't really like this location in the code... is there a better one?
There was a problem hiding this comment.
I don't like the process either. Saving all bridges right before loading them seems counterintuitive. ;) But let's speak about the process first, because as I mentioned it may not be a good idea ...
| @@ -45,6 +55,7 @@ function Cache() { | |||
| // For all discovered bridges | |||
| inBridges.forEach(inBridge => { | |||
| // Add new bridge to discovery object | |||
| log('discovered bridge: ' + inBridge.getID() + ' - ' + inBridge.getIP()); | |||
| if (globalSettings.bridges !== undefined) { | ||
| Object.keys(globalSettings.bridges).forEach(id => { | ||
| if (globalSettings.bridges[id].hasOwnProperty('ip')) { | ||
| log('restoring cached bridge: ' + id + ' - ' + globalSettings.bridges[id].ip); |
| this.saveCachedBridges = cache => { | ||
| let changed = false; | ||
| Object.keys(cache).forEach(id => { | ||
| if (globalSettings.bridges[id].ip != cache[id].ip) { |
There was a problem hiding this comment.
Weak typecheck. Also no check if globalSettings.bridges[id] is available on that object, might throw undefined errors.
| changed = true; | ||
| } | ||
| }); | ||
| if (changed) { |
| Object.keys(globalSettings.bridges).forEach(id => { | ||
| if (globalSettings.bridges[id].hasOwnProperty('ip')) { | ||
| log('restoring cached bridge: ' + id + ' - ' + globalSettings.bridges[id].ip); | ||
| discovery[id] = { 'ip': globalSettings.bridges[id].ip }; |
There was a problem hiding this comment.
This is js, not json. Don't use quotes for object properties. ;) And mybe use indention like the rest of the code.
| @@ -87,6 +87,9 @@ function connectElgatoStreamDeckSocket(inPort, inUUID, inRegisterEvent, inInfo, | |||
| else if (event === 'sendToPropertyInspector') { | |||
| // Save global cache | |||
| cache = jsonPayload; | |||
|
|
|||
| // Save Cached Bridges | |||
| pi.saveCachedBridges(cache); | |||
There was a problem hiding this comment.
I don't like the process either. Saving all bridges right before loading them seems counterintuitive. ;) But let's speak about the process first, because as I mentioned it may not be a good idea ...
| @@ -54,6 +54,20 @@ function PI(inContext, inLanguage, inStreamDeckVersion, inPluginVersion) { | |||
| document.getElementById('groups').label = instance.localization['GroupsTitle']; | |||
| } | |||
| }; | |||
|
|
|||
| // Save IP addresses for bridges into global settings | |||
| this.saveCachedBridges = cache => { | |||
There was a problem hiding this comment.
I don't get the use of that function. I mean, I get your intention. But this function replaces the IP in the global settings every time by the ip from the cache. I don't get why doing this is better. An this would inflict every dhcp connected bridge too, or not?
Addresses network connectivity issues discussed in #8