fixed subscriber bug in main library

[lokers]

more info about the bug here:
http://stackoverflow.com/questions/22442890/spacebrew-locally-arduino-yun-web-page-subscriber-not-working

and here:
https://groups.google.com/forum/#!topic/spacebrew-cc/uxzPQFqFN-4

I've tested on my machine and it seems like it's solving the problem, please review as authors if this is a good solution.
cheers

[robotconscience]

Hello! Can you re-submit your PR against the newest version? There are a few changes to the script to work better with node.

I actually think we can omit the line you've changed; it does look like we're checking in the block above if it's an admin or not. @julioterra and @quinkennedy can you confirm?

[julioterra]

The following line of code actually checks if the message was an admin
message:

if (!data.message["clientName"])

Standard non-admin messages should not include the attribute "clientName".
If changing this line is fixing the issues that seems to suggest that the
Spacebrew server is sending non-admin messages with this attribute.

Brett and Quin, have either of you updated the server recently in a way
that could have changed this?

Julio

On Thu, Mar 20, 2014 at 11:11 PM, Brett Renfer notifications@github.comwrote:

Hello! Can you re-submit your PR against the newest version? There are a
few changes to the script to work better with node.

I actually think we can omit the line you've changed; it does look like
we're checking in the block above if it's an admin or not. @julioterrahttps://github.com/julioterraand
@quinkennedy https://github.com/quinkennedy can you confirm?

Reply to this email directly or view it on GitHubhttps://github.com//pull/10#issuecomment-38245917
.

[lokers]

@julioterra It's either the sb-1.3.1.js that's outdated or the server libraries. they just don't match, perhaps the fix should be on the server side, but changing that line fixed the problem for me.
When I run my server in debug mode, all messages from my clients come in the format that actually has "clientName" property.
info: {"message":{"value":"227","type":"range","name":"brightness","clientName":"lokers-photoresistor-check","remoteAddress":"10.5.228.249"}}

When I removed the route via admin, this is what came through:
info: [{"route":{"type":"remove","client_disconnect":true,"subscriber":{"clientName":"lokers-test-html","name":"brightness","type":"range","remoteAddress":"10.5.203.66"},"publisher":{"clientName":"lokers-photoresistor-check","name":"brightness","type":"range","remoteAddress":"10.5.228.249"}}}]

@robotconscience , that's the latest version forked last night and pushed back with few minutes. Is there a newer somewhere?

[robotconscience]

@julioterra isn't the line before also checking for admin? @quinkennedy can you confirm?

@lokers hm, looking at your changes it still says upgrade from 1.2. Don't worry about it at the moment, I think we may be able to remove that line completely.

On Mar 21, 2014, at 7:21 AM, lokers notifications@github.com wrote:

@julioterra It's either the sb-1.3.1.js that's outdated or the server libraries. they just don't match, perhaps the fix should be on the server side, but changing that line fixed the problem for me.
When I run my server in debug mode, all messages from my clients come in the format that actually has "clientName" property.
info: {"message":{"value":"227","type":"range","name":"brightness","clientName":"lokers-photoresistor-check","remoteAddress":"10.5.228.249"}}

When I removed the route via admin, this is what came through:
info: [{"route":{"type":"remove","client_disconnect":true,"subscriber":{"clientName":"lokers-test-html","name":"brightness","type":"range","remoteAddress":"10.5.203.66"},"publisher":{"clientName":"lokers-photoresistor-check","name":"brightness","type":"range","remoteAddress":"10.5.228.249"}}}]

@robotconscience , that's the latest version forked last night and pushed back with few minutes. Is there a newer somewhere?

—
Reply to this email directly or view it on GitHub.

[robotconscience]

@julioterra @quinkennedy just a random thought, any chance it's just a bad bool comparison that chrome can understand but some node implementations don't?

On Mar 21, 2014, at 7:21 AM, lokers notifications@github.com wrote:

@julioterra It's either the sb-1.3.1.js that's outdated or the server libraries. they just don't match, perhaps the fix should be on the server side, but changing that line fixed the problem for me.
When I run my server in debug mode, all messages from my clients come in the format that actually has "clientName" property.
info: {"message":{"value":"227","type":"range","name":"brightness","clientName":"lokers-photoresistor-check","remoteAddress":"10.5.228.249"}}

When I removed the route via admin, this is what came through:
info: [{"route":{"type":"remove","client_disconnect":true,"subscriber":{"clientName":"lokers-test-html","name":"brightness","type":"range","remoteAddress":"10.5.203.66"},"publisher":{"clientName":"lokers-photoresistor-check","name":"brightness","type":"range","remoteAddress":"10.5.228.249"}}}]

@robotconscience , that's the latest version forked last night and pushed back with few minutes. Is there a newer somewhere?

—
Reply to this email directly or view it on GitHub.

[quinkennedy]

yes, all messages coming from the Server now include "clientName". This is a good example (for myself to learn from) of why you should not use implicit features to infer information. I'll review now.

[lokers]

Perhaps checking route vs message vs admin object will solve this... just a thought.
If you need help with testing of updated version, hit me up and I am happy to run it on my environment against my code. For now, I stick to my fix as it allows me to develop further, looking forward to official update. Cheers!

[robotconscience]

OK i'm going to knock out that line! Closing this PR, thanks again for figuring this out!