Agent Bug: bad callback for imp.wakeup: must take 0 parameters

It appears to help developers debug their code there may be a new error check in the Agent that takes a way a very critical feature from my perspective. I use the following style of codefor many of my imp.wakeup calls:

local test = 5 imp.wakeup(1.0, function(var = test){ server.log("INSIDE imp.wakeup = " + var) });

It allows me to pass a variable into a callback function that doesn’t take any arguments which is really handy compared to storing a list of variables for imp.wakeup callbacks somewhere in a global lookup table. When placed in Device code everything works fine and the imp logs:

INSIDE imp.wakeup = 5

This used to work inside of an Agent but now I get an error/code crash:

bad callback for imp.wakeup: must take 0 parameters

This is the first time I’ve noticed this - my agent is version 32dc179 - jenkins-ei-release-branch-718 - Tue Jul 16 14:46:44 2013.

This is the expected behaviour.

But fear not! What you want to accomplish is still possible through the magic of closures. Basically, we’re going write a factory function that takes in a parameter, and build / return a function that take NO parameters, but uses the parameter passed into the factory function.

function callbackFactory(var) return function() { server.log(var); }; }

This allows us to write code that looks like this:

local a = callbackBuilder("test1"); local b = callbackBuilder("test2"); a(); b();

The output of this will be:
test1 test2

Now that we can build a function that expects no parameters with a function that expects 1 (or more) parameters, we can use the result of our function in a method like imp.wakeup:

`function wakeupCallbackBuilder(var) {
return function() {
server.log("INSIDE imp.wakeup = " + var);
};
}

function test() {
local test = 5;
imp.wakeup(1.0, wakeupCallbackBuilder(test));
}

test();`

This can be a bit of a confusing concept… so if you have any questions let me know :slight_smile:

@beardedinventor is there any benefit to your way over mine (other than the squirrel crashing once the imp.wakeup is fired rather than when it is created)? I am a bit of a lazy programmer. =D

local test = 5 imp.wakeup(1.0, function(var = test){ server.log("INSIDE imp.wakeup = " + var) });

Feels a lot simpler, clearer as to what the imp.wakeup is doing from a code readability perspective, and possibly less “hacky” to me than

`function wakeupCallbackBuilder(var) {
return function() {
server.log("INSIDE imp.wakeup = " + var);
};
}

local test = 5;
imp.wakeup(1.0, wakeupCallbackBuilder(test));`

It’s at least less lines of code!

The advantage is that it works :slight_smile:

That said, when I looked the code again, I realized I made it unnecesarily complex. Because of how scope works, if you’re using a lambda function (an inline function), you don’t need to pass the variable into the function to use it… this should be enough:

local var = 5; imp.wakeup(1.0, function() { server.log("INSIDE imp.wakeup = " + var); });

My code worked up until today :wink: I was just trying to understand the design decision and was surprised because this was the first time I had working code that was broken by updated impOS code.

My code was a contrived example… I actually need a locally scoped variable (but I suppose I could just create a local copy right before my imp.wakeup function). In my case var is a global variable that may have changed by the time the imp.wakeup is triggered…

We put a lot of work into making non-breaking changes whenever we update functionality. Unfortunately, you stumbled onto some undocumented functionality that happened to be a bug (that was just squashed).

I’m not certain I fully understand what you’re actually trying to accomplish. If you can provide a bit more code, we can work through it?

See Line 513 of https://github.com/deldrid1/electric-imp-Hannah_Rev2/blob/master/Hannah_Rev2_Agent.nut.

Basically I have a LongPollListeners table that stores HTTP requests to an agent until a device triggers an event (such as a button press on line 720) that needs to be pushed down to clients. Before the request times out on the imp cloud (which I believe is currently limited to one minute) I’ d like to send a response each particular message.

It looks like by replacing

imp.wakeup(59.9, function(id = RequestID){

with

local id = RequestID; imp.wakeup(59.9, function(){

everything works. I didn’t realize that local variable outside a function would be in scope inside a callback function like that.

Yes, this is a bug really. Your function is callable with no parameters, so it ought to be valid as a wakeup callback.

How it worked previously, is that any function was accepted, but if it turned out not to be callable with no parameters, it produced a Squirrel run-time error at callback time with no line-number information. Which made it awkward to debug. So we changed it to check the parameter count when the wakeup was set, and throw an error if it was wrong. Looks like this check isn’t accounting for default parameters, which we didn’t realise. We didn’t think that the stricter checking would break any code except that which already didn’t work!

Peter

Thanks for the feedback Peter. If you can figure out a way to allow functions with default values and I can return using imp.wakeup the way I was before please let me know (it makes the code much more readable/understandable IMHO)!

In Squirrel, like Javascript, all identifiers in enclosing function scopes are accessible (and remain accessible even after the enclosing function in question has returned).

We will be fixing the bug and thus re-enabling your “default parameter” technique – we don’t like regressions round here. It wasn’t until I looked at the code that I appreciated that you wanted a separate callback for each value of “RequestID”. You’re right, the default-parameter technique is in fact quite a neat and expressive way of doing that. But in the meantime, beardedinventor’s callback-factory technique (which is also what I would have done) serves as a workaround.

Peter

@peter as a feature request could I ask that errors passed through a try{} catch(ex){} include the line number? Right now you can run into the same no line-number information if your error occurs in a try/catch, even if you log the exception…

Well yes you can ask for that as a feature request, but you don’t need to, because we already spotted that as a problem when writing our own Squirrel, and it’s on the list to get fixed!

Because of how Squirrel works you won’t get the full backtrace (through calling functions), but you will at least get the line number where it occurred.

Peter

@peter - looks like I just encountered the same thing using sendasync():

http.post(postURL, headers, out_message).sendasync(function(response, url=postURL){

spits out the error:
bad 'done' callback to request.sendasync: must take 1 parameter

Is this something that will be changed back to the way it was as well or do I need to change my code? Looking through one of the broken Agents it looks like its a strategy I used for just about every callback function in my code…

The fix is in, and will take effect the next time we deploy new server code.

Peter