Esp32 cleanup jswarn

[MaBecker]

for details see comments with first commit

[gfwilliams]

Otherwise this looks good though - it'll be nice to avoid those annoying messages

[MaBecker]

Thanks for checking

[MaBecker]

Please do not merge yet, will let you when ready to merge

[MaBecker]

@gfwilliams can you please share how to handle data for callback functions.

Is this how to?

  JsVar *tmp = jsvNewObject();

  // Schedule callback if a function was provided
  jsvObjectSetChildAndUnLock(tmp, "hostname", jsvNewFromString(hostname));
  if (jsvIsFunction(jsCallback)) {
    JsVar *params[2];
    params[0] = jsvNewWithFlags(JSV_NULL);
    params[0] = tmp;
    jsiQueueEvents(NULL, jsCallback, params, 1);
    jsvUnLock(params[0]);
  }

[gfwilliams]

Yes, that looks good - but some typos in there....

• the second params[0] should be [1]?
• jsiQueueEvents last argument should be 2 because 2 arguments
• tmp should be unlocked at the end of the function in the scope it was allocated in

[MaBecker]

I have no clue what

params[0] = jsvNewWithFlags(JSV_NULL);

is for

[gfwilliams]

Did that code come from somewhere? All that line does is make the first argument null - we were trying to keep the API the same between different ways to connect Espruino to the net, and also to keep it with a Node.js-ish API.

In some Node.js APIs they have a callback with an error argument. It's null of there's no error, or a String if there is an error: https://github.com/espruino/Espruino/blob/8f3a9cb52/libs/network/wiznet/jswrap_wiznet.c#L325

[MaBecker]

Yes, that looks good - but some typos in there....

• the second params[0] should be [1]?
• jsiQueueEvents last argument should be 2 because 2 arguments
• tmp should be unlocked at the end of the function in the scope it was allocated in

like this?

JsVar *tmp = jsvNewObject();

  // Schedule callback if a function was provided
  jsvObjectSetChildAndUnLock(tmp, "hostname", jsvNewFromString(hostname));
  if (jsvIsFunction(jsCallback)) {
    JsVar *params[2];
    params[0] = jsvNewWithFlags(JSV_NULL);
    params[1] = tmp;
    jsiQueueEvents(NULL, jsCallback, params, 2);
    jsvUnLock(params[1]);
  }
  jsvUnLock(tmp);

[gfwilliams]

Nope - you changed jsvUnLock(params[0]); to jsvUnLock(params[1]); so now you're not unlocking the first arg but are unlocking the second one twice.

This should be fine:

JsVar *tmp = jsvNewObject();

  // Schedule callback if a function was provided
  jsvObjectSetChildAndUnLock(tmp, "hostname", jsvNewFromString(hostname));
  if (jsvIsFunction(jsCallback)) {
    JsVar *params[2];
    params[0] = jsvNewWithFlags(JSV_NULL);
    params[1] = tmp;
    jsiQueueEvents(NULL, jsCallback, params, 2);
    jsvUnLock(params[0]);
  }
  jsvUnLock(tmp);

[gfwilliams]

So do you think this is all ready now?

[gfwilliams]

Great - thanks!

[MaBecker]

Thanks for checking, tried to fix your findings. Would say good to go for now.