Skip to content
This repository was archived by the owner on Nov 25, 2024. It is now read-only.

Cache and restore bridge IP addresses into global settings#30

Open
jaeichel wants to merge 7 commits intoelgatosf:masterfrom
jaeichel:jaeichel-patch-save-bridge-ips
Open

Cache and restore bridge IP addresses into global settings#30
jaeichel wants to merge 7 commits intoelgatosf:masterfrom
jaeichel:jaeichel-patch-save-bridge-ips

Conversation

@jaeichel
Copy link

Addresses network connectivity issues discussed in #8

// 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;
Copy link
Author

Choose a reason for hiding this comment

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

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);
Copy link
Author

Choose a reason for hiding this comment

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

I don't really like this location in the code... is there a better one?

Copy link
Contributor

Choose a reason for hiding this comment

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

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());
Copy link
Contributor

Choose a reason for hiding this comment

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

Use template strings.

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Use template strings.

this.saveCachedBridges = cache => {
let changed = false;
Object.keys(cache).forEach(id => {
if (globalSettings.bridges[id].ip != cache[id].ip) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Weak typecheck. Also no check if globalSettings.bridges[id] is available on that object, might throw undefined errors.

changed = true;
}
});
if (changed) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Space missing. ;)

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 };
Copy link
Contributor

Choose a reason for hiding this comment

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

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

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 => {
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants