Skip to content
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

this.sendSocketNotification is not a function #196

Closed
saml-dev opened this issue Apr 18, 2016 · 81 comments
Closed

this.sendSocketNotification is not a function #196

saml-dev opened this issue Apr 18, 2016 · 81 comments
Labels

Comments

@saml-dev
Copy link

I have just about finished my Traffic module to get current commute time, but I'm having difficulty sending a socket notification other than the initial one in my start function. The project can be found here and the issue lies in MMM-Traffic.js.

I send a socket notification in start which queries the google maps api in node_helper.js then sends a socket notification back to MMM-Traffic.js which will then call updateDom(). I'm using updateCommute() as a proxy for setInterval that will send the initial socket notification at the user defined interval, but I am getting an error saying that this.sendSocketNotification is not a function. Any ideas?

@paviro
Copy link
Contributor

paviro commented Apr 18, 2016

Really interested in your module! I am not sure (I am glad my Wunderlist module works at all). I think it has something to do with the setInterval(this.updateCommute, this.config.interval * 1000);. Have a look at my Wunderlist node_helper.js I had to pass this into the function called by setInterval in order for it to work.
In my code self == this at the point where the timer is executed.

My timer:
setInterval(self.updateData, self.config.interval, data, self, i);

My function:

updateData: function (data, self, i) {
    self.get_tasks(data[self.config.lists[i].replace(/\s+/g, '')].id, self.config.lists[i], function (data, list_id, list_name) {

      self.sendSocketNotification('WUNDERLIST_TASKS', {list_id: list_id, tasks: data, list_name: list_name});

     })
  },

@MichMich
Copy link
Collaborator

On my phone, so not a well formatted reply. The reason of your issue is that within the setTimout closure, this no longer points to your module instance. This is a common thing in JavaScript. To solve this, create a helper variable before your create the timer:

var self = this;

And then in the timer closure use:

self.sendSocketNotificaction(......

@paviro
Copy link
Contributor

paviro commented Apr 18, 2016

Would this be the timer inside of the setTimout closure?

@MichMich
Copy link
Collaborator

MichMich commented Apr 18, 2016

I think so, yes.

You can easily find out by logging this.

Log.log(this);

@MichMich
Copy link
Collaborator

And just to be clear. You can of course use any helper variable name. I prefer 'self' because I do a lot of Swift programming. But a lot of people use 'var that=this;' in JavaScript.

@saml-dev
Copy link
Author

Many thanks to both of you! This is my first time using Node so I used your Wunderlist module frequently as a reference (and as a starting point). I just tried adding this as a parameter to the function and I'm no longer getting the original error, but the notification doesn't seem to be making it's way back to MMM-Traffic.js. Is there a way to log/see information inside of node_helper?

@paviro
Copy link
Contributor

paviro commented Apr 18, 2016

Use console.log(), output will appear in the terminal session you started the mirror in. Never thought that module would be used as reference :D Still hoping for someone with actually JavaScript knowledge to improve it a bit..

@MichMich
Copy link
Collaborator

The Log.log helper does not work in the node helper as you might have noticed. :)

console.log(aVariable);

Should work.

Note: to send a notification from the node helper to the front end, the frontend first needs to send a message (to open the connection.)

@MichMich
Copy link
Collaborator

Check this module for simple socket messages: https://github.com/MichMich/mmm-systemtemperature

@saml-dev
Copy link
Author

Probably should have thought to just try the default console.log() :P
I send the first notification from front end to node helper each time so that shouldn't be an issue, but it doesn't seem like the notification is ever reaching the node helper except for the initial one sent in start.

@saml-dev
Copy link
Author

saml-dev commented Apr 18, 2016

Scratch that, it is reaching node_helper. So the issue must be somewhere in there. I'll keep looking.

@saml-dev
Copy link
Author

Double scratch that, only the first one is reaching node helper. Losing my mind over here :P

@MichMich
Copy link
Collaborator

Can you post your code somewhere? So I can take a quick look? (Still on my phone, so I can only point you in the right direction.)

@saml-dev
Copy link
Author

@MichMich
Copy link
Collaborator

Ah, the way you use it, you shouldn't use self. (Only within a closure this is nessecery)

So only the sendSocketNotification in start is correctly fired. The others probably generate an error. Check your web inspector console.

@MichMich
Copy link
Collaborator

(I'd really love to explain it better. But typing on a phone is a b*tch)

@paviro
Copy link
Contributor

paviro commented Apr 18, 2016

Works in my module that way :/ At least I am doing the same thing in the Node helper script.

@saml-dev
Copy link
Author

I almost wish they were generating errors because then I'd have a place to look, but I'm not getting any errors on the console. I'm confused why it works in start but not in updateCommute because it's the same line of code that sends the socketNotification.

@saml-dev
Copy link
Author

Well not the exact same line but a duplicate.

@MichMich
Copy link
Collaborator

Also, the way you are currently building it, you can only use the module once. If you want to show two routes by adding the module to your config twice with a different destination, you're gonna have a bad time. ;)

@MichMich
Copy link
Collaborator

No, on one spot u use self. In start u use this. In this case: use this.
Also add some logging to see if your method is being called.

@saml-dev
Copy link
Author

Right but self is just a proxy for this that I could pass to the function right?

@saml-dev
Copy link
Author

saml-dev commented Apr 18, 2016

The method in the interval is being called, and I logged self.url to make sure that passing this as self was functioning correctly.

@MichMich
Copy link
Collaborator

On the place where you use self, self does not exist. It only exists in start.

@MichMich
Copy link
Collaborator

Hmmm that's weird.

@saml-dev
Copy link
Author

I passed self as a parameter at the end of the setIntervall call.

@MichMich
Copy link
Collaborator

You are making it unnecessary difficult. Try this:

updateCommute: function() {

    this.sendSocketNotification('TRAFFIC_URL', this.url);
},

@saml-dev
Copy link
Author

That's what I had originally, and that's when I got the this.sendSocketNotification is not a function error.

@saml-dev
Copy link
Author

Which left me very confused after an hour of digging, hence my post here :)

@MichMich
Copy link
Collaborator

Ok ... you did it: you got me out of bed! ;)
One second. Will look into it,

@saml-dev
Copy link
Author

I've got to go to class so I'll have to mess with it some more later. Get some sleep! Thanks again for the help.

@saml-dev
Copy link
Author

I'll let you know if I am able to figure anything out.

@MichMich
Copy link
Collaborator

Ok, the issue is solved: there was a huge bug that caused the issue that only the last configured module could keep sending socketNotifications. This is solved now.

I close this issue, but feel free to keep using this issue if you need any help building your module.

@saml-dev
Copy link
Author

Great! Everything is working now. Thanks again for your help!

@saml-dev
Copy link
Author

Okay, I've updated it to work for multiple instances of the module and everything is working based on my tests. Please check it out and let me know if you have any suggestions!

I was thinking of adding an option to show the destination in small, dim text on a line underneath the current view, in case people want to be able to easily differentiate between routes if they have multiple. What do you both (or anyone else reading this) think?

@saml-dev
Copy link
Author

Also here is another link to the code so you don't have to go searching through the conversation above: https://github.com/SamLewis0602/MMM-Traffic

@pvyParts
Copy link

the address might be a bit long but it could be dimmed out nickname of the locations like the card in google now
"Home to Work"

or could do both if no "nickname" is set for the origin/dest use the address

( i've not tested the module yet but will tonight )

@saml-dev
Copy link
Author

Ah that's a good idea. Maybe just something like a routeName field that will allow them to call the route whatever they want. Then there would be no need for a showDestination, but rather I could just show the routeName in the DOM if it exists. Thanks!

@saml-dev
Copy link
Author

Okay, support for route_name in the config file has been added. I'll add it to the README now. Let me know what you think!

@CFenner
Copy link
Contributor

CFenner commented Apr 20, 2016

@paviro: If in the handler itself you don't need the original this you can override it like this.

setTimeout(
  this.handler.bind(this),
  this.config.updateIntervall
);

or

setTimeout(
  function(){
    //doSomething
  }.bind(this),
  this.config.updateIntervall
);

@MichMich
Copy link
Collaborator

Great work @SamLewis0602! I'm using your module on my mirror. My girlfriend asked me for this features a few days ago! :)

I ended you a pull request with multi language support. This way it can be used in other languages as well.

I'll continue further question/issues in your repro te keep everything in one place. :)

@paviro
Copy link
Contributor

paviro commented Apr 20, 2016

Don't forget to add your module to the wiki, so that others can find it! Will give it a try later, thanks for your work!

Am 20.04.2016 um 09:58 schrieb Michael Teeuw notifications@github.com:

Great work @SamLewis0602! I'm using your module on my mirror. My girlfriend asked me for this features a few days ago! :)

I ended you a pull request with multi language support. This way it can be used in other languages as well.

I'll continue further question/issues in your repro te keep everything in one place. :)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@paviro
Copy link
Contributor

paviro commented Apr 20, 2016

@CFenner will try that thanks!

@kapsolas
Copy link
Contributor

kapsolas commented Jul 15, 2016

I seem to be having a similar issue that I can't figure out..

Just setup the Mirror and looking to write some modules. Trying to get the notifications to work before I start building out code and it isn't working.

In my module.js file I do the following

in the start method I do the following


start: function () {
    Log.info ('Starting module: ' + this.name);
   this.url = ''; // I have a method that generats the URL and it works ok

   this.grabPhotos();
},

grabPhotos: function() {
  self.sendSocketNotification("MyNotification", this.url);
}

In the node_helper.js the code is as follows


var NodeHelper = require('node_helper');

module.exports = NodeHelper.create( {
  start: function() {
    console.log("Starting helper module");
  },

  socketNotificationReceived: function(notification, payload) {
    console.log("notification received");
  }
});

I never see any notifications reached.. I even tried to pass this in grabPhotos() and then use self, but it did not work... Stumped for a few hours. not sure what the issue is.

Thanks!

EDIT: I installed the mmm-systemtemperature module also to step through it. it does not work either. Was something messed up in the last update? I did a git pull and it states I have the latest.

@MichMich
Copy link
Collaborator

You are running sendSocketNotification on self., but it doesn't seem like you initialized the self var. try this.sendSocketNotification .

@kapsolas
Copy link
Contributor

I have tried with this as well with no difference.

To make sure I didn't have any code issues. I installed the mmm-systemtemperature module, and it does not work either. Making me thing maybe its something else?

@MichMich
Copy link
Collaborator

What version of node are you running?

@kapsolas
Copy link
Contributor

The version I have is v6.2.1

@MichMich
Copy link
Collaborator

Do all the other default modules work?

@kapsolas
Copy link
Contributor

Yes. thus far, all the other default modules are working.

I even modified the hello world as a start to display an image using just an to ensure its working. That worked fine.

I updated the config and added new RSS feeds to the newsfeed, that also worked fine.

@MichMich
Copy link
Collaborator

Could shrare your repository somewhere so I can try?

@kapsolas
Copy link
Contributor

Sure!

I made a pastebin with both the files since I don't have a repo set up for it yet on github.

The link is good for 24 hrs. after that I set it to expire.
http://pastebin.com/GHpg5J9X

At this point, the UI is not wired up. I'm just trying to get the notifications to work and checking for messages in the console.

Once I get that working, I will be coding it up to parse an RSS feed from Flickr and pull images to rotate through.

thanks for your help!

@kapsolas
Copy link
Contributor

No need to try the repository. Not sure what the problem was, but I
rebooted the raspberry pi zero and my PC and stuff started to work..

Part of the my issue was that I expected the console.log to write to the
Console log in firebug, but it does not. It literally goes to the console
where you kick-off the process!

My module is complete at this time, it loads images from Flickr and
animates them!

If people are interested in this, I can upload it to my github.

Thanks!!

On Sat, Jul 16, 2016 at 10:26 AM, Michael Teeuw notifications@github.com
wrote:

Could shrare your repository somewhere so I can try?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#196 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAwk5opiSnPnFyImMiZX1_TB60Fswx3Bks5qWPgygaJpZM4IKFMz
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants