Skip to content

Add serial transport#232

Open
trond-snekvik wants to merge 1 commit into
mainfrom
transport/serial
Open

Add serial transport#232
trond-snekvik wants to merge 1 commit into
mainfrom
transport/serial

Conversation

@trond-snekvik

Copy link
Copy Markdown
Collaborator

Adds the serial transport layer, as outlined in golioth/architecture#85. This does not include a real transport adapter, but includes three test suites that test the serial device, serial broker and a test that covers both together, each using mock serial adapters and endpoints.

There's some cleanup and some hacking in the kconfig menusystem going into this PR. The intent is to just kinda make serial build on both the gateway and the device without making our menuconfig worse. I'll make a separate PR with some more deliberate restructuring.

@trond-snekvik trond-snekvik force-pushed the transport/serial branch 4 times, most recently from 2badb17 to 53bfd2d Compare June 11, 2026 10:10
Adds the serial transport layer, as outlined in golioth/architecture#85.
This does not include a real transport adapter, but includes three test
suites that test the serial device, serial broker and a test that covers
both together, each using mock serial adapters and endpoints.

Signed-off-by: Trond Snekvik <trond.snekvik@canonical.com>
Comment on lines -11 to -15
config POUCH_TRANSPORT_NONE
bool "NONE"
help
No transport is selected. Useful for unit testing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need this for building unit tests without the dependencies of a given transport.

Comment on lines -41 to -46
config POUCH_TRANSPORT_ACK_TIMEOUT_MS
int "Pouch Transport ACK timeout (ms)"
default 1000
help
The timeout, in milliseconds, that a receiver will wait
for new packets before retransmitting an acknowledgement

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think symbol is used by sar/receiver.c

Comment on lines +52 to +55
config POUCH_TRANSPORT_SAR
bool
help
Selected by transports that require Segmentation and Reassembly (SAR)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎉

Comment on lines +170 to +172
pouch_atomic_clear_bit(&broker->flags, FLAG_SYNC);
pouch_atomic_clear_bit(&broker->flags, FLAG_DOWNLINK_DONE);
pouch_atomic_clear_bit(&broker->flags, FLAG_UPLINK_DONE);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to clear the FLAG_INFO_READ flag here?


void pouch_serial_broker_destroy(struct pouch_serial_broker *broker)
{
free(broker);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need any kind of teardown (ie: call end()) or check to ensure the broker isn't still in use?

pouch_serial_ch_ready(&broker->serial.channels[ch]);
}

static void next(struct pouch_serial_broker *broker)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a neat pattern 🎉

}

/* First prompt: open the transfer. */
if (!pouch_atomic_test_and_set_bit(&ch->flags, CH_FLAG_OPEN))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This prevents opening a transfer when one is already open. Is it okay to silently return 0 when that happens?

};

*buf = pouch_serial_header_encode(&hdr);
pouch_atomic_clear_bit(&ch->flags, CH_FLAG_ERROR);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's tricky to understand why the error flag is cleared here.


void pouch_serial_device_init(pouch_serial_device_ready_cb_t cb)
{
ready_cb = cb;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we save this because it's a singleton? I'm wondering because the transport will also store it via the init command on the next line.

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.

2 participants