Skip to content
This repository has been archived by the owner on Oct 5, 2024. It is now read-only.

Message received only once? #11

Closed
ygalanter opened this issue Mar 7, 2019 · 8 comments
Closed

Message received only once? #11

ygalanter opened this issue Mar 7, 2019 · 8 comments
Labels
bug Something isn't working

Comments

@ygalanter
Copy link

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?

@ygalanter
Copy link
Author

I believe the issue might be here since ID coming generated on companion is always the same int this case - seed, the condition is true only once.

@daveyjones daveyjones added the bug Something isn't working label Mar 7, 2019
@daveyjones
Copy link
Contributor

daveyjones commented Mar 7, 2019

@ygalanter the ID is supposed to be updated for each newly enqueued message:

const get_next_id = () => {
const last_msg = get_queue().slice(-1)[0]
if (last_msg && last_msg._asap_id) {
return last_msg._asap_id + 1
} else {
return seed
}
}

@Artaud do you want to take a look at this and see what might be going on?

@ygalanter
Copy link
Author

It looks like when there's nothing in the queue (guess it happens when all messages sent?) it returns the seed.

@daveyjones
Copy link
Contributor

I think we need to move line 5 to line 71 so a new seed is generated every time the queue is emptied. I'll submit a fix in a few minutes.

daveyjones added a commit that referenced this issue Mar 7, 2019
@daveyjones
Copy link
Contributor

I added a test and confirmed the behavior. @Artaud if you pull the latest from GitHub you can run the test and see. The message is sent the first time you change the input text in the settings view but messages are not sent for subsequent changes. Unfortunately, I need to jump over to another project so I don't have time right now to finish troubleshooting. I'll revisit in the next couple days if @Artaud hasn't addressed it by then.

Artaud added a commit to Artaud/fitbit-asap that referenced this issue Mar 8, 2019
@Artaud
Copy link
Collaborator

Artaud commented Mar 8, 2019

Thanks for spotting this! We can't just generate a new random seed whenever the queue empties up. We still need the message IDs to be incremental, since we check if the last received ID is greater than the previous (on

if (data._asap_id > last_received_message_id) {
).
So either we need to ensure that the newly generated seed is greater than the previousy generated, or we can simply remember the last generated ID.
This is a little tricky considering that we can have the queue persistent from previous session.

So cases are:

  1. start of the app with no queue > get_next_id() gives you a random seed
  2. during runtime, queue empties up. Enqueueing a new message should get ID of the previous message + 1
  3. start of the app with queue. This is the tricky part since if all of the messages in the queue are sent before another is enqueued, we lose the information about the last id, thus granting on average a 50% chance that no further messages will be received.
    So we need to check the queue on startup and save the last id.

I got a PR

Artaud added a commit to Artaud/fitbit-asap that referenced this issue Mar 8, 2019
@daveyjones
Copy link
Contributor

@ygalanter, version 1.1.1 is now available via npm. I added a basic test which now passes thanks to the PR made by @Artaud. I'll close this for now but please feel free to re-open or create a new issue if you run into anything else!

@ygalanter
Copy link
Author

Awesome! Thank you so much for such fast turnaround.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants