Skip to content

Conversation

Mattemagikern
Copy link
Contributor

Aims to address: #96860

Comment on lines +115 to +116
struct ring_buffer *rx_ringbuf;
struct ring_buffer *tx_ringbuf;
Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason this breaks my CDC ACM echo stress tester. I generally run a pseudorandom (with a known seed) sequence and expect the CDC ACM sample to echo it back. Prior to this PR, everything works fine. With this PR, after resetting the board, I keep getting mismatch always at byte 64512. I have 1024 byte long synchronization pattern before the pseudorandom sequence. 64512+1024=65536 seems to suggest that there is some sort of wrapping issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi!

Thanks for reviewing and testing!

Could you share the test with me?
I wrote this test but it didn't catch it. I tested with various size of both input & ring_buffer size.
The test should surpass the uint16_t overflow and would then hopefully trigger the behavior you found.

/*
USB_CDC_ACM_RINGBUF_SIZE = 1024
#define CDC_ACM_BUFFER_SIZE (CONFIG_CDC_ACM_BULK_EP_MPS)
CDC_ACM_BUFFER_SIZE == CDC_ACM_BULK_EP_MPS = 64 or 512
*/
static uint8_t big_buff[1024];
static uint8_t big_input[66*1024];
static uint8_t big_output[66*1024];
void fill_big_input(void)
{
	sys_rand_get(big_input, sizeof(big_input));
}

ZTEST(ringbuffer_api, test_stress_big)
{
	struct ring_buffer rb;
	ring_buffer_init(&rb, big_buff, sizeof(big_buff));
	fill_big_input();

	size_t read = 0;
	size_t written = 0;
	size_t to_write;
	size_t to_read;
	uint8_t *ref;
	while (read < sizeof(big_input)) {
		to_write = MIN(ring_buffer_write_ptr(&rb, &ref), sizeof(big_input) - written);
		zassert_true(to_write > 0, "there should be space in the buffer");
		memcpy(ref, &big_input[written], to_write);
		ring_buffer_commit(&rb, to_write);
		written += to_write;

		while (read < written) {
			to_read = ring_buffer_read_ptr(&rb, &ref);
			zassert_true(to_read > 0, "there should be space in the buffer");
			memcpy(&big_output[read], ref, to_read);
			ring_buffer_consume(&rb, to_read);
			read += to_read;
		}

	}
	LOG_HEXDUMP_DBG(&big_input[64*1024], 64, "big_input");
	LOG_HEXDUMP_DBG(&big_output[64*1024], 64, "big_output");
	zassert_mem_equal(big_input, big_output, sizeof(big_input), "read != written");
}

The ring buffer api has been overhauled to provide a more reabable, smaller
code size, and more efficient implementation. The new api is not backwards
compatible.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
Introducing basic tests for the new ring_buffer API.
Moving the existing tests for the deprecated ringbuffer API to a separate
directory ensuring both can be maintained and run independently.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
Move to new ring_buffer api.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
move to new ring_buffer api.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
move to new ring_buffer api.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
move to use new ring_buffer API.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
move to new ring_buffer api.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
Move to new ring_buffer api.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
Move to new ring_buffer API.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
Move to use new ring_buffer api.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
Update various subsystems to use the new ring_buffer API.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
samples to use new ring_buffer api.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
Move to use new ring_buffer_api.

Signed-off-by: Måns Ansgariusson <Mansgariusson@gmail.com>
Copy link

sonarqubecloud bot commented Oct 4, 2025

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