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

OLS Improvements #51

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hardware/openbench-logic-sniffer/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi)
return SR_ERR;

/* Reset all operational states. */
devc->rle_count = devc->num_transfers = 0;
devc->rle_count = devc->raw_sample_buf_size = 0;
devc->num_samples = devc->num_bytes = 0;
devc->cnt_bytes = devc->cnt_samples = devc->cnt_samples_rle = 0;
memset(devc->sample, 0, 4);
Expand Down
53 changes: 28 additions & 25 deletions src/hardware/openbench-logic-sniffer/protocol.c
Original file line number Diff line number Diff line change
Expand Up @@ -393,41 +393,27 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
serial = sdi->conn;
devc = sdi->priv;

if (devc->num_transfers == 0 && revents == 0) {
if (devc->cnt_bytes == 0 && revents == 0) {
/* Ignore timeouts as long as we haven't received anything */
return TRUE;
}

if (devc->num_transfers++ == 0) {
devc->raw_sample_buf = g_try_malloc(devc->limit_samples * 4);
if (!devc->raw_sample_buf) {
sr_err("Sample buffer malloc failed.");
return FALSE;
}
/* fill with 1010... for debugging */
memset(devc->raw_sample_buf, 0x82, devc->limit_samples * 4);
}

num_changroups = 0;
for (i = 0x20; i > 0x02; i >>= 1) {
if ((devc->capture_flags & i) == 0) {
num_changroups++;
}
}

if (revents == G_IO_IN && devc->num_samples < devc->limit_samples) {
if (revents == G_IO_IN) {
if (serial_read_nonblocking(serial, &byte, 1) != 1)
return FALSE;
devc->cnt_bytes++;

/* Ignore it if we've read enough. */
if (devc->num_samples >= devc->limit_samples)
return TRUE;

devc->sample[devc->num_bytes++] = byte;
sr_spew("Received byte 0x%.2x.", byte);
if (devc->num_bytes == num_changroups) {
unsigned int samples_to_write;
unsigned int samples_to_write, new_sample_buf_size;

devc->cnt_samples++;
devc->cnt_samples_rle++;
Expand Down Expand Up @@ -491,6 +477,28 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
}

samples_to_write = devc->rle_count + 1;
new_sample_buf_size =
4 * MAX(devc->limit_samples,
devc->num_samples + samples_to_write);

if (devc->raw_sample_buf_size < new_sample_buf_size) {
unsigned int old_size =
devc->raw_sample_buf_size;
new_sample_buf_size *= 2;
devc->raw_sample_buf =
g_try_realloc(devc->raw_sample_buf,
new_sample_buf_size);
devc->raw_sample_buf_size = new_sample_buf_size;

if (!devc->raw_sample_buf) {
sr_err("Sample buffer malloc failed.");
return FALSE;
}
/* fill with 1010... for debugging */
memset(devc->raw_sample_buf + old_size, 0x82,
new_sample_buf_size - old_size);

Choose a reason for hiding this comment

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

I wonder if we still need that? Also 0x82 is not 1010...

From my point of view, the benefits of having a dynamic buffer and, thus, supporting RLE fully is bigger than the disadvantage of having problems with high-latency network connections. I didn't dive in fully, but the code looks reasonable.

Brainstorming: would it be possible to add a driver-option 'use static buffers' to support high latencies? Not requesting it for this series, maybe it can be added on top. From a glimpse, I'd think this is possible.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, indeed, the pattern is wrong. I still consider it useful, though, so I changed it to 0xAA. Especially the most recent version of this file contains changes for a different number of channel groups than 4, which I didn't consider when doing these changes in the first place.

I'm happy that you also find RLE support more worthwhile. Ideally, we would know when to finish reading, but at least 2021 me didn't know how.

About having both buffer options: At this point, I would refrain from adding more options to the code, as this means adding more ways in which things could break. I'd think that looking again into whether there is a way to figure out the buffer size in the first place could be worthwhile instead. I agree that I won't change it as part of this series.

}

for (i = 0; i < samples_to_write; i++)
memcpy(devc->raw_sample_buf +
(devc->num_samples + i) * 4,
Expand Down Expand Up @@ -539,10 +547,7 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
packet.payload = &logic;
logic.length = devc->trigger_at_smpl * 4;
logic.unitsize = 4;
logic.data = devc->raw_sample_buf +
(devc->limit_samples -
devc->num_samples) *
4;
logic.data = devc->raw_sample_buf;
sr_session_send(sdi, &packet);
}

Expand All @@ -560,13 +565,11 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
logic.length =
(devc->num_samples - num_pre_trigger_samples) * 4;
logic.unitsize = 4;
logic.data = devc->raw_sample_buf +
(num_pre_trigger_samples + devc->limit_samples -
devc->num_samples) *
4;
logic.data = devc->raw_sample_buf + num_pre_trigger_samples * 4;
sr_session_send(sdi, &packet);

g_free(devc->raw_sample_buf);
devc->raw_sample_buf = 0;

serial_flush(serial);
abort_acquisition(sdi);
Expand Down
2 changes: 1 addition & 1 deletion src/hardware/openbench-logic-sniffer/protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ struct dev_context {
int trigger_at_smpl;
uint16_t capture_flags;

unsigned int num_transfers;
unsigned int num_samples;
int num_bytes;
int cnt_bytes;
Expand All @@ -124,6 +123,7 @@ struct dev_context {
unsigned int rle_count;
unsigned char sample[4];
unsigned char *raw_sample_buf;
unsigned int raw_sample_buf_size;
};

SR_PRIV extern const char *ols_channel_names[];
Expand Down