dillpixel / fitbit-asap Goto Github PK

For some reason the method get_queue is sometimes returning 0 instead of an array, @gregoiresage believes it's because of lack of available space on device.

App: Unhandled TypeError: Expected a function.
  queue at node_modules/fitbit-asap/app.js:16,5
  enqueue at node_modules/fitbit-asap/app.js:47,5
  asap at node_modules/fitbit-google-analytics/app.js:43,5
  send$1 at node_modules/fitbit-google-analytics/app.js:55,9

(Credit to @gregoiresage for helping me debug and find more info about the issue)

Adding a few lines (As suggested by @gregoiresage) seemed to fix this issue for me (at least for the small time I've been testing it) but I don't think it is a good permanent fix:

var get_queue = function () {
    try {
        var queue = readFileSync("_asap_queue", "cbor");
        if (!Array.isArray(queue)) {
            return [];
        }
        return readFileSync("_asap_queue", "cbor");
    }
    catch (error) {
        return [];
    }
};

In some applications, it is desirable that the queue be cleared when the app exits. I think we should either provide a flag for such behaviour or provide a method to clear the persisted queue.

What should the callback on message expiration return?
I'm thinking either message text, or message text and the creation timestamp? The timestamp however won't be under the library user's control, potentially making it unusable for anything practical.

I just found in my app that I needed to check that a certain message was indeed acked and then do something based on that event.
I wrote a callback for that for myself, should I add it to fitbit-asap as well?

Found a bug to squash!

In the send_next() method in both app and companion, when we get an error twice, there is no mechanism to schedule another send.
On first error, we schedule send in the send() method, but when we get an error even then, we just stop.

Pushing a fix

At

Fix incoming.

I have a Google Pixel and a Fitbit Versa.

On the fitbit, a call to peerSocket.send is used as a means to wake the connection between the device and app. If no send is generated, connectivity is left at the will of the device (I think).

The result was that messages were being queued, but not sent in a timely manner. I addressed this by adding a ping method which I call on startup. It looks like this:

const pingConnection = (timeout = 1000) => {
  // make a ping method which
  // sleeps for a timeout
  // then sends a message
  const ping = () => {
    console.log("ping");
    setTimeout(() => {
      peerSocket.send("ping");
    }, timeout);
  };

  // add a listener to the error event
  // which will ping again when after an error
  peerSocket.addEventListener('error', () => {
    ping();
  });

  // send the first ping
  ping();
};

Effectively this will send pings every second until there's a successful connection at which point it will not resend. This forces the connection to establish, though will have a battery penalty should the connection be unavailable.

Would you consider something like this for fitbit-asap?

Aside from file storage, the code on the app and the companion is identical. I would be nice to consolidate everything that the app and companion have in common into a file that can be imported by each. I think I tried that originally and ran into an issue—I can't remember what it was—but it might be worth exploring again.

I'm thinking that there are way too many write/read operations on filesystem involved currently.

Right now, to send one message, we have two reads and one write on the sending side, and one read and one write on the receiving side.
That's for every message, and we always read and write the entire queue. The IO operations are synchronous, so there is a possibility of freezing the app if we have a larger queue.

Could we make the IO calls asynchronous?

Hi @daveyjones, this is more of a suggestion, something I'd need and would be willing to work on.

From the documentation on the MessageSocket.send() in the Fitbit SDK, I understood that the peer messaging channel between companion and watch is not 100% reliable, quoting:

There is no message loss while the endpoints are connected. This means that there are no gaps. A message is either successfully transmitted, in order, or it failed to be delivered because the channel has been closed (endpoints disconnected). After a failure, the channel is closed and no messages can be exchanged until a new channel is opened. It is possible, however, that the last messages sent before disconnection will never be delivered.

If I understand that correctly, it means I can never trust messaging.peerSocket.readyState.

I need a 100% reliable channel, which would mean implementing ACK message queue. Is this something that you would like to see in this library? I'm currently going to implement this in my app anyway, so I figured it might as well go into a public module.

I think it would be a really nice fit to add acks to your messaging library.

According to this post, it should be possible to run Jest tests with Fitbit apps.

I would subscribe to some test suite since otherwise we are probably not going to be able to test some more in-depth scenarios (like enqueing multiple messages with simulated intermittent connectivity), at least not in a repeatable way.

Not sure if this is an actual issue or if I am doing something wrong (though I've used fitbit-asap in many other projects with no issues). I am sending message from companion to the app on settings change. On the app asap.onmessage fires only once, when I change setting the first time. Consecutive changes have no effect on the app even though the message is sent from companion. I think this might've started when I upgraded to SDK 3.1. Any idea what could be the reason?

Currently, the in-memory queue and the on-disk queue are identical, and messages will continue to be added to both queues until the device (a) runs out of memory or (b) runs out of storage. This needs to be managed. First, we'll need to create a mechanism for detecting these limits. Then, we'll need to prune messages in order to stay within the limits. It probably makes sense to prune the oldest messages and then fire an onerror callback. It may also make sense to only store a portion of the queue in memory, with the full queue stored on disk.

See issue #2 for previous discussion on this topic.