Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsolicited Status Response contains 0xFF "command processed" code #10

Closed
lumagi opened this issue Nov 29, 2020 · 6 comments · Fixed by #15
Closed

Unsolicited Status Response contains 0xFF "command processed" code #10

lumagi opened this issue Nov 29, 2020 · 6 comments · Fixed by #15

Comments

@lumagi
Copy link
Contributor

lumagi commented Nov 29, 2020

When running the LogAndStream firmware with an active Bluetooth connection to the Shimmer, the device sends unsolicited status response packets when its status changes. One example is when the Shimmer is placed in a dock. The corresponding function in the code in main.c prepends the response with a 0xFF byte, which signals that a command has been processed by the Shimmer. However, since the status response is unsolicited, this response code is invalid and should not be send.

@marknolan
Copy link
Member

Hi Lumagi,

You have correctly identified a feature of the LogAndStream FW that we call "In-stream" responses. The advantage of this is that we can get live feedback of the Shimmer3 while streaming in software like Consensys. For example, if the Shimmer is docked while streaming, a little dock image shows on the Shimmer in Consensys. Similarly, it allows us to poll the Shimmer for it's battery voltage so that we can give live feedback of it without having to stop/start streaming.

The disadvantage obviously is that, each time a packet is received, the API needs to check whether it's an in-stream response or part of the data stream and parse it appropriately.

Our Java API (i.e., Consensys) handles three instream responses:

  1. DIR_RESPONSE = 0x88
    This isn't really used any more. It was originally implemented to give live feedback as to which directory/bin file the Shimmer3 was currently recording to on it's SD card.

  2. STATUS_RESPONSE = 0x71
    1 byte status representing:
    image

  3. VBATT_RESPONSE = 0x94
    3 bytes representing battery voltage adc value (bytes 0 and 1 in LSB order) and the charging status (I can supply more info if needed)

@lumagi
Copy link
Contributor Author

lumagi commented Dec 14, 2020

I must apologize if my issue description was not concise enough. The issue does not relate to the responses themselves but to the acknowledgment byte that is prepended to the unsolicited status responses. To make my point more clear, I want to compare a regular status request to the unsolicited status response triggered by the device.

The communication flow for a regular status request looks as follows

--> 0x72
<-- 0xFF
<-- 0x8A 0x71 0xAB

where the device answers with an acknowledgment byte 0xFF to acknowledge the request, the instream response type 0x8A, the status response type 0x71, and the actual value 0xAB.

However, when the device sends an unsolicited status response due to a docking event, the following bytes are sent by the device:

(Event on the device)
<-- 0xFF
<-- 0x8A 0x71 0xAB

This sequence of bytes also features the acknowledgment byte 0xFF to acknowledge a prior command request. However, since the sequence was not triggered by a prior command, I believe it should not contain the acknowledgment byte. Additionally, having the acknowledgment byte sent by the device generates and ambiguity in the input state machine on the host. It is not possible to know if the acknowledgment was actually sent in response to a command that should be acknowledged or due to an event that happended on the device. Only taking into account the state of the host and the acknowledgment byte in the input stream, it is not possible to decide if the state machine should read more data from the stream and block for it (i.e. expecting a status response) or only read the single byte and acknowledge a previously sent command.

My suggestion would be to remove the acknowledgment byte for the unsolicited status response such that the data sent by the device looks like sthis:

(Event on the device)
<-- 0x8A 0x71 0xAB

This change only concerns the unsolicited status response, not the regular status request and response. Please also take a look at the pull request I made. It features the necessary changes.

@marknolan
Copy link
Member

marknolan commented Sep 21, 2021

@lumagi I think you are correct with your assessment about the 0xFF - it looks like it was intentionally added - maybe as an extra layer of protection to distinguish it from streaming packets. The difficulty now is that all of our APIs have been developed to look out for the 0xFF if the device is streaming and we don't have any plans to update them at present.

From looking at our JAVA API, I think the 0xFF is effectively skipped over if the device is streaming (i.e., tracked by a boolean in the code) and the subsequent byte is a valid in-stream response byte - I know this is a messy solution but maybe you could repeat it in your code.

@marknolan
Copy link
Member

@lumagi to satisfy both sides, we could implement a new BT command to provide the ability to disable the undesired 0xFF. In FW we can leave the 0xFF enabled by default (for compatibility with our other software) and then, during the connection process with the Python API, the new command could be called to turn it off. After disconnection the 0xFF would be turned back on. What do you think?

I'm hoping we can provide a way for the 'vanilla' version of LogAndStream to work with the pyshimmer API.

@lumagi
Copy link
Contributor Author

lumagi commented Feb 16, 2023

Hi Mark,
that sounds like a good plan. I would also prefer the vanilla firmware to be compatible with the API. I can implement the necessary changes on the Python side.

@marknolan
Copy link
Member

@lumagi I've created #15 to handle this update and I've included a pre-packaged FW in the PR description which can be used for testing.

@marknolan marknolan linked a pull request Feb 17, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants