Skip to content

Remove serial signalling protocol from producers to consumers#305

Merged
Ivan-Velickovic merged 1 commit intomainfrom
serial_producer_signal_unconditional
Jan 30, 2025
Merged

Remove serial signalling protocol from producers to consumers#305
Ivan-Velickovic merged 1 commit intomainfrom
serial_producer_signal_unconditional

Conversation

@Courtney3141
Copy link
Contributor

@Courtney3141 Courtney3141 commented Jan 13, 2025

Currently, both the consumer and producer of a serial queue only signal each other if their corresponding flags are set - there are two flags for each queue. This is in contrast to the networking subsystem, where the consumer never signals the producer, and the producer only signals the consumer depending on the value of the flag.

Since the serial subsystem is using a signalling protocol for both directions of communication, it makes the code quite complicated to read and understand for those who are unfamiliar with the protocol. Since the serial subsystem is not performance critical, and the risk of over signalling even with unconditional signalling is very low, I have decided to remove one direction of the signalling protocol so that producers of serial queues signal consumers unconditionally after a batch of work has been enqueued.

The consumer side of the protocol remains unchanged, as typically producers do not require signals from consumers - it is only required when a queue has become full and there is still more data to be enqueued. Since under normal conditions this does not occur frequently, it makes sense to keep the protocol for this direction of communication.

Merge after #301.

@Courtney3141 Courtney3141 force-pushed the serial_producer_signal_unconditional branch from 1949d59 to 872542c Compare January 13, 2025 04:15
@Courtney3141 Courtney3141 force-pushed the serial_producer_signal_unconditional branch from 872542c to 2dec068 Compare January 13, 2025 04:23
@JE-Archer JE-Archer changed the base branch from main to simplify_serial_queue_interface January 14, 2025 07:13
@JE-Archer JE-Archer changed the base branch from simplify_serial_queue_interface to split_serial_rx_tx_clients January 14, 2025 07:13
@Courtney3141 Courtney3141 force-pushed the serial_producer_signal_unconditional branch from 2dec068 to 079bbdc Compare January 15, 2025 07:13
@Courtney3141 Courtney3141 changed the base branch from split_serial_rx_tx_clients to simplify_serial_queue_interface January 15, 2025 07:15
@Courtney3141 Courtney3141 force-pushed the simplify_serial_queue_interface branch from 524ca66 to 41b35a2 Compare January 15, 2025 07:16
@Courtney3141 Courtney3141 force-pushed the serial_producer_signal_unconditional branch 2 times, most recently from 23a7090 to 0df8f55 Compare January 15, 2025 07:26
@Courtney3141 Courtney3141 force-pushed the simplify_serial_queue_interface branch from 41b35a2 to d8bed51 Compare January 17, 2025 06:58
@Courtney3141 Courtney3141 force-pushed the serial_producer_signal_unconditional branch from 0df8f55 to ea468fb Compare January 17, 2025 07:02
@Courtney3141 Courtney3141 force-pushed the simplify_serial_queue_interface branch from d52a7d8 to bdb9b0b Compare January 25, 2025 02:07
alwin-joshy
alwin-joshy previously approved these changes Jan 28, 2025
Base automatically changed from simplify_serial_queue_interface to main January 30, 2025 06:03
@Ivan-Velickovic Ivan-Velickovic dismissed alwin-joshy’s stale review January 30, 2025 06:03

The base branch was changed.

Signed-off-by: Courtney Darville <courtneydarville94@outlook.com>
@Ivan-Velickovic Ivan-Velickovic force-pushed the serial_producer_signal_unconditional branch from ea468fb to 42d5e9a Compare January 30, 2025 06:04
@Ivan-Velickovic Ivan-Velickovic enabled auto-merge (rebase) January 30, 2025 06:06
@Ivan-Velickovic Ivan-Velickovic merged commit f20e452 into main Jan 30, 2025
7 checks passed
@Ivan-Velickovic Ivan-Velickovic deleted the serial_producer_signal_unconditional branch January 30, 2025 06:07
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