On Fri, Aug 11, 2017 at 10:18 AM, Jef Driesen jef@libdivecomputer.org wrote:
On 2017-08-10 19:32, John Van Ostrand wrote:
On Thu, Aug 10, 2017 at 11:00 AM, Jef Driesen jef@libdivecomputer.org
So this means that the byte sequences:
05 9D FF 00 43 00 05 BD 7F 00 43 00
that are send in the cochran_commander_read_id() function are actually read operations at the addresses 0xFF9D and 0x7FBD (and a size of 0x0043 bytes). Interesting. I didn't realize that before.
It seems so obvious, but yes, that's exactly what it means. It didn't dawn on me until I had to get creative reading the Commander TM.
Having less "magic values" is always nice!
// Read the sample data, from 0 to sample end will include logbook
rc = cochran_commander_read (device, &progress,device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), device->layout->rb_profile_end);
// Read the sample data, logbook and sample data are contiguousrc = cochran_commander_read (device, &progress,device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), size); if (rc != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to read the sample data."); return rc;
This can't work. The size variable is calculated as:
layout->rb_profile_end - layout->rb_logbook_begin
For the Commander TM that is 0x010000 bytes (0x020000 - 0x010000). But the size field in the command is only 16 bits wide. So you'll try to read 0 bytes. Oops. I think the only solution here is to split the large read operation into multiple smaller blocks.
I thought so too but when I ask for ask for 0 bytes it returns 32K.
That's a bit weird, but if it works, fine. The rationale behind it is probably that doing a read operation for 0 bytes makes little sense, so they can re-use that value to mean 0x10000. In that case we should probably add some extra checks:
if (size > 0x10000) return DC_STATUS_INVALIDARGS;
if (size == 0) return DC_STATUS_SUCCESS;
and some comment saying that 0 is interpreted by the device as 0x10000. Otherwise someone will ask the same question in the future. And by that time we probably forgot about that detail :-)
Good idea.