Proper testing

[Hexagon]

I have not found time to properly test each function in this library, is there someone who is up to the task?

[kmpm]

I will continue the work that I started and hopefully be able to finish. There is one thing though. The method renaming fix in #13 doesn't really document the callback signatures. In the readme there is a returnValue in the examples. What does this returnValue represent and what is the second argument that is returned, at least for some of them, as well? When calling turnOff the first argument is always 0 and the second is always 1. turnOn gives 0 on the first and 0 on the second. What are these ... hard to make tests not knowing.

[Hexagon]

Great!

All functions that are not expected to return a string do return an integer representing an internal code from telldus, for example TELLDUS_SUCCESS = 0, TELLDUS_DEVICE_NOT_FOUND=-4 etc.

These messages can be decoded with getErrorString(returnValue,function(errorString) {}). Note that getErrorString returns 'Success' for returnValue 0, so the name of it is a little wierd.

All callbacks should only get one argument passed back to them, except the three listener callbaks which has special signatures. If they actually get two arguments, ignore the second one, i would guess it's some internal V8-crap.:)

[Hexagon]

Oh, and there's a massive bugfix for return values in 0.0.4, so make sure you use that or a newer version when testing.

[kmpm]

Oh, good. I'll try to make some tests that will return something else then 0 as well then.

I found out about the second argument, It's the Worktype from the call to AsyncCaller. Is that something you plan to "hide"? I can perhaps imagine it useful if you used one single callback function for all your async calls. That way you could see what method that triggered this callback. It's an unlikely scenario and I'll ignore it for the moment but it should perhaps be mentioned somewhere if someone else wonders.

[nilzen]

I think it may be useful to keep the "WorkType" but I'm not sure of the data type and naming here. Isn't a string with the function name more intuitive?

0 -> "turnOn" 1 -> "turnOff" etc

[Hexagon]

Aah, now i see why the worktype is returned :) Yeah i guess we could convert the id's to names further on, but I'll leave it as numbers until further notice.

[kmpm]

I'll try to list the status in this comment and update them as time goes. I would consider this issue closed when all these methods have some kind of test on them.

Methods that have tests either by them selves or as a part of another test. In order of appearance.

telldus.js

• [x] addDeviceEventListener
• [ ] addSensorEventListener -- moved to separate issue
• [x] addRawDeviceEventListener
• [x] getDevicesSync
• [x] turnOn
• [x] turnOff
• [x] dim
• [x] learn
• [x] addDevice
• [x] setName
• [x] getName
• [x] setProtocol
• [x] getProtocol
• [x] setModel
• [x] getModel
• [x] getDeviceType
• [x] removeDevice
• [x] removeEventListener
• [x] getErrorString
• [x] turnOnSync
• [x] turnOffSync
• [x] dimSync
• [x] learnSync
• [x] addDeviceSync
• [x] setNameSync
• [x] getNameSync
• [x] setProtocolSync
• [x] getProtocolSync
• [x] setModelSync
• [x] getModelSync
• [x] getDeviceTypeSync
• [x] removeDeviceSync
• [x] removeEventListener
• [x] getErrorStringSync
• [x] nodeAsyncCaller

[Hexagon]

Awesome work, didn't know you had come so far already!

[Hexagon]

4 new functions wrapped! (Both sync and async)

exports.getNumberOfDevices = function(callback) { return nodeAsyncCaller(17, 0, 0, "", callback); }
exports.stop = function(id, callback) { return nodeAsyncCaller(18, id, 0, "", callback); }
exports.bell = function(id, callback) { return nodeAsyncCaller(19, id, 0, "", callback); }
exports.getDeviceId = function(id, callback) { return nodeAsyncCaller(20, id, 0, "", callback); }

getDeviceId takes deviceIndex and converts it to deviceId, used for iterating from 0 to getNumberOfDevices()

[kmpm]

Those are somewhat dangerous I can imagine. What happens if some other software adds or removes devices after getNumberOfDevices is called but before getDeviceId?

[Hexagon]

Yep, the developer have to be careful with that, the normal procedure in this case should be (psuedo-code)

for ( deviceIndex = 0 ; deviceIndex  < getNumberOfDevices() ; deviceIndex++ ) {
   deviceId = getDeviceId(deviceIndex)
}

Hence should getNumberOfDevices never be cached in the developers application.

To be entirely safe, we could include en extra check in telldus.cc that validate the parameter of getDeviceId to be between 0 and getNumberOfDevices()-1

[nilzen]

No harm in exposing getDeviceId but its very "core" so if anyone uses it they should know what they are doing, so I don't think you need to add extra checks.

[Hexagon]

5 new functions wrapped @36e3e0d

exports.getDeviceParameterSync = function (id, name, val) { return telldus.getDeviceParameter(id, name, val); };
exports.setDeviceParameterSync = function (id, name, val) { return telldus.setDeviceParameter(id, name, val); };
exports.executeSync = function (id) { return telldus.execute(id); };
exports.upSync = function (id) { return telldus.up(id); };
exports.downSync = function (id) { return telldus.down(id); };

And Async equivalents for all of then.

[kmpm]

Please add any new functions to a issue of it's own, preferably not all in one but in several with perhaps related methods. Otherwise this one will never close... I will close this issue when the original list of methods is covered.

[Hexagon]

Sure! All methods of telldus api is implemented now so the light in the end of the tunnel is starting to shine :)

[kmpm]

I'm closing this one but have removed addSensorEventListener from here and added to #49