-
Notifications
You must be signed in to change notification settings - Fork 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.
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
stopID caching to prevent excessive findStop calls #8
Conversation
node_helper.js
Outdated
}); | ||
} | ||
}); | ||
if (!self.stop.id || self.stop.name != payload.stopName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please verify how the module behaves in case multiple instances are configured? As per my understanding NodeHelper is a single instance per module type, i.e. notifications from different modules with potentially different stop ids will be received. Since the stop variable in the NodeHelper contains a single value it will be flipping and, finally, no calls to dvb.findStop will be avoided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Working on it.
So, now it seems to be solved and tested on two modules simultaneously.
|
Thanks for your contribution! |
The current implementation (IMHO) generates excessive requests to
dvb.findStop()
method, as it is not really required to search for a station ID during every update request.As per my understanding, we need to get
stopID
when:Therefore, I added intermediate storing mechanism for
stopID
andstopName
.stopID
searching is triggered only in above mentioned two cases, otherwise onlydvb.monitor()
is triggered.