Cache and restore bridge IP addresses into global settings#30
Cache and restore bridge IP addresses into global settings#30jaeichel wants to merge 7 commits intoelgatosf:masterfrom
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
| 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 ...
| // 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.
| 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 ...
| }; | ||
|
|
||
| // 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