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

Add support for Mares Sirius #65

Merged
merged 6 commits into from
Jul 24, 2024

Conversation

rmultan
Copy link
Contributor

@rmultan rmultan commented Jul 23, 2024

Cherry pick changes from libdivecomputer to
allow support for Mares Sirius computer.

@mikeller
Copy link
Member

What are the changeset id(s) that you cherry-picked from https://github.com/libdivecomputer/libdivecomputer for this?

@rmultan
Copy link
Contributor Author

rmultan commented Jul 23, 2024

@mikeller To be exact it's not a cherry pick anymore (as if git cherry pick), I just took code from libdivecomputer to make it work, because first time I did it I couldn't build Subsurface on my M1 Mac. I made another attempt. I'm not familiar with C, so it was a bit of a struggle to fix merge issues, so I opted to pick manually what I needed.

Relevant commits from libdivecomputer libdivecomputer/libdivecomputer@4516842...8fe598e

@mikeller
Copy link
Member

@rmultan: Unfortunately not having the original commits cleanly cherry-picked and the additional changes in a separate commit will create merge conflicts for all future merges from the libdivecomputer upstream. So merging this in its current form is not a good option.
I was planning to do an upstream merge from libdivecomputer in the near future anyway, so we can probably just wait until this will bring in the changes for the Mares Sirius.

jefdriesen and others added 6 commits July 24, 2024 08:37
Devices supporting BLE 4.2 (or higher) can use data packets with a
payload size of up to 244 bytes. None of the current Mares models
support this at the moment, but increasing the size on the receiving
side already prepares the code for future models and remains fully
backwards compatible.

Signed-off-by: rmultan <[email protected]>
The macros make the code a bit more compact, and adding support for new
models becomes easier too.

Signed-off-by: rmultan <[email protected]>
The Mares Genius (and compatible models) uses a new command to download
different types of objects, instead of manually reading and parsing the
flash memory. This command also supports reading device properties like
the serial number.

This change is necessary because newer models like the Sirius no longer
support reading directly from the flash memory.

Signed-off-by: rmultan <[email protected]>
The current implementation assumes a fixed order for the different
record types to share some code with the older models. See commits [1]
and [2] for more details. The main disadvantages of this approach are
the extra complexity and the limited flexibility to adapt to future
changes to the data format.

To make the parsing code more future proof, split the code into separate
functions for the Genius/Horizon and the older models.

[1] Commit 8b06f2c (Horizon)
[2] Commit feec939 (Genius)

Signed-off-by: rmultan <[email protected]>
The Mares Sirius uses the same communication protocol as the Genius,
except for the fact that it uses a newer BLE version which supports
larger data packets. The actual MTU is likely negotiated because we see
different sizes like 244 and 182 bytes. We don't have access to this MTU
size because not all BLE implementations can expose this information.

Unfortunately not only the BLE packet size is variable, but also the
size of the higher level data frames (used for downloading the content
of the objects) is no longer fixed. The frame size appears to adapt to
the BLE MTU size. This is most likely done to reduce the overhead and
maximize the throughput.

Although each frame ends with an OxEA byte, we can't rely on this
knowledge to detect the end of the frame. The END byte is not escaped in
the payload, and thus can also appear anywhere in the frame. As a
workaround, we rely on the fact that each frame appears to be send as a
single BLE packet. The only exception is the ACK byte, which gets send
as a separate BLE packet if the command requires parameter data.

Compatible models: Quad Ci, Puck 4 and Puck Air 2

Signed-off-by: rmultan <[email protected]>
@rmultan rmultan reopened this Jul 24, 2024
@rmultan
Copy link
Contributor Author

rmultan commented Jul 24, 2024

@mikeller Updated with an actual cherry pick. If it works for you I'm happy to update the version in the main repository as well, if not I'll just wait for your merge of the upstream.

Copy link
Member

@mikeller mikeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you for contributing!

@mikeller mikeller merged commit 710fc40 into subsurface:Subsurface-DS9 Jul 24, 2024
15 checks passed
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 this pull request may close these issues.

3 participants