On 2017-07-15 22:39, John Van Ostrand wrote:
static dc_status_t -cochran_commander_read_config (cochran_commander_device_t *device, dc_event_progress_t *progress, unsigned char data[], unsigned int size) +cochran_commander_read_config (cochran_commander_device_t *device, dc_event_progress_t *progress, unsigned char data[])
Removing the size parameter is a bad idea. Any function that takes some buffer as argument should also have a corresponding size parameter. Even if it's only used to verify whether the caller passed the right amount:
if (size % 512 != 0) return some_error;
If you simply trust the caller to always pass a buffer of the right size, you'll run into trouble someday!
dc_device_t *abstract = (dc_device_t *) device; dc_status_t rc = DC_STATUS_SUCCESS;
- unsigned int pages = 2;
- if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM)
pages = 1;- // Read two 512 byte blocks into one 1024 byte buffer
- for (unsigned int i = 0; i < 2; i++) {
const unsigned int len = size / 2;
for (unsigned int i = 0; i < pages; i++) {
unsigned char command[2] = {0x96, i};
rc = cochran_commander_packet(device, progress, command,sizeof(command), data + i * len, len, 0);
unsigned int command_size = 2;if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM)command_size = 1;rc = cochran_commander_packet(device, progress, command,command_size, data + i * 512, 512, 0); if (rc != DC_STATUS_SUCCESS) return rc;
How about using the size argument to calculate the number of pages? That way the function becomes even more generic. Only the caller needs to know how many pages there are. The function just needs to handle the difference between the 1 or 2 byte command variant (and even that can be based on the number of pages).
if (device->layout->baudrate == 9600) {// Older commander, use low-speed read commandcommand[0] = 0x05;command[1] = (address ) & 0xff;command[2] = (address >> 8) & 0xff;command[3] = (address >> 16) & 0xff;command[4] = (size ) & 0xff;command[5] = (size >> 8 ) & 0xff;command_size = 6;} else {// Newer commander with high-speed read commandcommand[0] = 0x15;command[1] = (address ) & 0xff;command[2] = (address >> 8) & 0xff;command[3] = (address >> 16) & 0xff;command[4] = (size ) & 0xff;command[5] = (size >> 8 ) & 0xff;command[6] = (size >> 16 ) & 0xff;command[7] = 0x04;command_size = 8;}
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.
- // 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 contiguous
- rc = 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.
Jef