Skip to content

sat: timeout: Add timeouts for sat transmission #230

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.

Sign up for GitHub

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

Open
ceolin wants to merge 6 commits into
base: main
Choose a base branch
from
Open

sat: timeout: Add timeouts for sat transmission #230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0603b09
sat: nrf52: Set timeouts for transmissions
ceolin Jun 8, 2026
055e7f6
sat: nrf53: Set timeouts for transmissions
ceolin Jun 8, 2026
003d79b
sat: nrf54: Set timeouts for transmissions
ceolin Jun 8, 2026
b3fa937
sat: efr32: Set timeouts for transmissions
ceolin Jun 8, 2026
41ddcbf
sat: esp32: Set timeouts for transmissions
ceolin Jun 8, 2026
81ef957
sat: ti: Set timeouts for transmissions
ceolin Jun 8, 2026
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
11 changes: 11 additions & 0 deletions include/hubble/port/sat_radio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,17 @@ extern "C" {
#define HUBBLE_SAT_PREAMBLE_SEQUENCE \
(const int8_t[]){63, 0, 63, 0, 63, 0, 63, 63}


/**
* @brief Timeout, in seconds, to wait to start a transmission.
*
* Transmissions are serialized so that only one transmission runs at
* a time. This value bounds how long a caller waits to acquire that
* start a transmission before giving up, preventing a stuck or
* long-running transmission from blocking other callers indefinitely.
*/
#define HUBBLE_SAT_TRANSMISSION_TIMEOUT_S 2U

/**
* @brief Initialize the satellite radio port.
*
Expand Down
22 changes: 19 additions & 3 deletions port/esp-idf/hubblenetwork-sdk/boards/esp32c6/radio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
#define HUBBLE_BASE_FREQUENCY 2482U
#define HUBBLE_CHANNEL_OFFSET(_channel) (((_channel) * 64) + 489)

#define _TIME_US_TO_TICK(_time_us) ((_time_us / 1000U) / portTICK_PERIOD_MS)
#define _TIME_S_TO_TICK(_time_s) ((_time_s * 1000U) / portTICK_PERIOD_MS)
Comment on lines +40 to +41

@HongNguyen635 HongNguyen635 Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I kind of feeling something off about this so I asked Claude and checked with ESP-IDF source:

#define portTICK_PERIOD_MS          ( ( TickType_t ) 1000 / configTICK_RATE_HZ )

-> portTICK_PERIOD_MS = 1000 / 100 = 10

where tick rate Hz is 100 by default. I also doubled check the build folder and the default of CONFIG_FREERTOS_HZ is indeed 100.

So say we have symbol off = 800 us -> (800 / 1000U) / 10 = 0
or symbol on = 8000 us -> ( 8000 / 1000 ) / 10 = 0

So either case it still fails.

@ceolin ceolin Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yep, good point, we should have at least 1 tick as min.


/**
* This semaphore is used to protect a packet transmission and avoid
* race conditions.
Expand Down Expand Up @@ -195,17 +198,25 @@ static int _radio_cw_start(uint16_t step, uint32_t delay, uint32_t duration_us)
return ret;
}

xSemaphoreTake(_symbol_sem, portMAX_DELAY);
ret = !(xSemaphoreTake(_symbol_sem, _TIME_US_TO_TICK(2 * duration_us)) ==
pdTRUE);

/* Symbol off time */
phy_tx_tone(false, true, DEFAULT_TX_POWER_DBM);

if (ret != 0) {
return -ETIMEDOUT;
}

ret = _timer_start(delay);
if (ret != 0) {
return ret;
}

xSemaphoreTake(_symbol_sem, portMAX_DELAY);
if (xSemaphoreTake(_symbol_sem, _TIME_US_TO_TICK(2 * delay)) != pdTRUE) {
return -ETIMEDOUT;
}

return 0;
}

Expand Down Expand Up @@ -273,7 +284,12 @@ int hubble_sat_board_packet_send(const struct hubble_sat_packet_frames *packet)
int ret = 0;
int8_t frame = -1;

xSemaphoreTake(_transmit_sem, portMAX_DELAY);
if (xSemaphoreTake(_transmit_sem,
_TIME_S_TO_TICK(HUBBLE_SAT_TRANSMISSION_TIMEOUT_S)) !=
pdTRUE) {
return -ETIMEDOUT;
}

xSemaphoreTake(_symbol_sem, 0); /* Reset semaphore */

for (uint8_t i = 0; i < packet->total_number_of_symbols; i++) {
Expand Down
9 changes: 8 additions & 1 deletion port/freertos/boards/ti/cc23xx_cc27xx/radio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
/* Hubble */
#include <hubble/port/sat_radio.h>
#include <hubble/sat/packet.h>
#include <hubble/port/sat_radio.h>
Comment on lines 25 to +27

@HongNguyen635 HongNguyen635 Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

duplicate include?


/* DMM */
#if defined(USE_DMM_OVRDE)
Expand Down Expand Up @@ -61,6 +62,8 @@
#error "Device not supported"
#endif

#define _TIME_S_TO_TICK(_time_s) ((_time_s * 1000U) / portTICK_PERIOD_MS)

@HongNguyen635 HongNguyen635 Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm a bit curious on is this preferred over pd_MS_TO_TICKS( )? I recall something about using pd_MS_TO_TICKS have some safety guard like promotion and type cast.


/**
* This semaphore is used to protect a packet transmission and avoid
* race conditions.
Expand Down Expand Up @@ -231,7 +234,11 @@ int hubble_sat_board_packet_send(const struct hubble_sat_packet_frames *packet)
{
int8_t frame = -1;

xSemaphoreTake(_transmit_sem, portMAX_DELAY);
if (xSemaphoreTake(_transmit_sem,
_TIME_S_TO_TICK(HUBBLE_SAT_TRANSMISSION_TIMEOUT_S)) !=
pdTRUE) {
return -ETIMEDOUT;
}

for (uint8_t i = 0; i < packet->total_number_of_symbols; i++) {
int16_t step;
Expand Down
23 changes: 17 additions & 6 deletions port/zephyr/boards/nordic/nrf52/nrf52_soc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@
#include <stdint.h>
#include <errno.h>

#define RADIO_NODE DT_NODELABEL(radio)
#define RADIO_NODE DT_NODELABEL(radio)

/* From NRF52840_PS_v1.2 6.20.15.8 Time between TXEN -> READ is 140us */
#define WAIT_SYMBOL_OFF_US (HUBBLE_WAIT_SYMBOL_OFF_US - 140)
#define WAIT_SYMBOL_US (HUBBLE_WAIT_SYMBOL_US + 140)
#define WAIT_SYMBOL_OFF_US (HUBBLE_WAIT_SYMBOL_OFF_US - 140)
#define WAIT_SYMBOL_US (HUBBLE_WAIT_SYMBOL_US + 140)

/* Max time for semaphore symbol to wait before failing. */
#define WAIT_SYMBOL_TIMEOUT_US K_USEC(2 * (WAIT_SYMBOL_OFF_US + WAIT_SYMBOL_US))

@HongNguyen635 HongNguyen635 Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is the 2 here arbitrary? Or is it related to the 2 uses in HUBBLE_SAT_TRANSMISSION_TIMEOUT_S?

@HongNguyen635 HongNguyen635 Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure how to feel about the use of constant 2 through the code. When we decide to change it to another number later, we'll have to go through the port and change each. Would it makes sense if we define something for it, e.g. SYMBOL_TIMEOUT_FACTOR or something like that...

@ceolin ceolin Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it is arbitrary, just two times the expected time, could be even less.

Let me re-struct it and make it consistent


static uint32_t _radio_shorts;

Expand Down Expand Up @@ -168,9 +171,14 @@ int hubble_sat_soc_disable(void)

int hubble_sat_soc_packet_send(const struct hubble_sat_packet_frames *packet)
{
int ret;
int8_t frame = -1;

k_sem_take(&_transmit_sem, K_FOREVER);
ret = k_sem_take(&_transmit_sem,
K_SECONDS(HUBBLE_SAT_TRANSMISSION_TIMEOUT_S));
if (ret != 0) {
return ret;
}

k_sem_reset(&_symbol_sem);

Expand All @@ -186,15 +194,18 @@ int hubble_sat_soc_packet_send(const struct hubble_sat_packet_frames *packet)

hubble_nrf_lib_frequency_set(packet->frame[frame].channel,
packet->frame[frame].data[data_pos]);
k_sem_take(&_symbol_sem, K_FOREVER);
ret = k_sem_take(&_symbol_sem, WAIT_SYMBOL_TIMEOUT_US);
if (ret != 0) {
break;
}
}

_ppi_disable();
_timer_disable();

k_sem_give(&_transmit_sem);

return 0;
return ret;
}

#ifdef CONFIG_HUBBLE_SAT_NETWORK_DTM_MODE
Expand Down
24 changes: 18 additions & 6 deletions port/zephyr/boards/nordic/nrf53/nrf53_soc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,17 @@

#endif /* CONFIG_HUBBLE_SAT_NETWORK_DTM_MODE */

#define RADIO_NODE DT_NODELABEL(radio)
#define RADIO_NODE DT_NODELABEL(radio)

/**
* Time between TXEN task and READY event with fast ramp-up enabled is
* 40us. nRF5340_PS_v1.6 - 7.26.16.8
*/
#define WAIT_SYMBOL_OFF_US (HUBBLE_WAIT_SYMBOL_OFF_US - 40)
#define WAIT_SYMBOL_US (HUBBLE_WAIT_SYMBOL_US + 40)
#define WAIT_SYMBOL_OFF_US (HUBBLE_WAIT_SYMBOL_OFF_US - 40)
#define WAIT_SYMBOL_US (HUBBLE_WAIT_SYMBOL_US + 40)

/* Max time for semaphore symbol to wait before failing. */
#define WAIT_SYMBOL_TIMEOUT_US K_USEC(2 * (WAIT_SYMBOL_OFF_US + WAIT_SYMBOL_US))

#define RADIO_ENABLE_TX_ON_CC0_PPI 9U
#define RADIO_DISABLE_ON_CC1_PPI 12U
Expand Down Expand Up @@ -231,9 +234,15 @@ int hubble_sat_soc_enable(void)

int hubble_sat_soc_packet_send(const struct hubble_sat_packet_frames *packet)
{
int ret;
int8_t frame = -1;

k_sem_take(&_transmit_sem, K_FOREVER);
ret = k_sem_take(&_transmit_sem,
K_SECONDS(HUBBLE_SAT_TRANSMISSION_TIMEOUT_S));
if (ret != 0) {
return ret;
}

k_sem_reset(&_symbol_sem);

_dppi_enable();
Expand All @@ -248,15 +257,18 @@ int hubble_sat_soc_packet_send(const struct hubble_sat_packet_frames *packet)

hubble_nrf_lib_frequency_set(packet->frame[frame].channel,
packet->frame[frame].data[data_pos]);
k_sem_take(&_symbol_sem, K_FOREVER);
ret = k_sem_take(&_symbol_sem, WAIT_SYMBOL_TIMEOUT_US);
if (ret != 0) {
break;
}
}

_dppi_disable();
_timer_disable();

k_sem_give(&_transmit_sem);

return 0;
return ret;
}

#ifdef CONFIG_HUBBLE_SAT_NETWORK_DTM_MODE
Expand Down
26 changes: 19 additions & 7 deletions port/zephyr/boards/nordic/nrf54/nrf54_soc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@
#include <stdio.h>
#include <stdlib.h>

#define RADIO_NODE DT_NODELABEL(radio)
#define RADIO_NODE DT_NODELABEL(radio)

/* From nRF54L15 PS: Time between TXEN -> READY is ~40us with fast ramp-up */
#define WAIT_SYMBOL_OFF_US (HUBBLE_WAIT_SYMBOL_OFF_US - 40)
#define WAIT_SYMBOL_US (HUBBLE_WAIT_SYMBOL_US + 40)
#define WAIT_SYMBOL_OFF_US (HUBBLE_WAIT_SYMBOL_OFF_US - 40)
#define WAIT_SYMBOL_US (HUBBLE_WAIT_SYMBOL_US + 40)

#define NRF_DPPIC NRF_DPPIC10
/* Max time for semaphore symbol to wait before failing. */
#define WAIT_SYMBOL_TIMEOUT_US K_USEC(2 * (WAIT_SYMBOL_OFF_US + WAIT_SYMBOL_US))

#define NRF_DPPIC NRF_DPPIC10
#define RADIO_ENABLE_TX_ON_CC0_PPI 9U
Comment on lines +39 to 40

@HongNguyen635 HongNguyen635 Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

another weird case from the formatter :(

@ceolin ceolin Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yep, super annoying. I can try to revert it and see if CI complains

#define RADIO_DISABLE_ON_CC1_PPI 12U

Expand Down Expand Up @@ -216,9 +219,15 @@ int hubble_sat_soc_enable(void)

int hubble_sat_soc_packet_send(const struct hubble_sat_packet_frames *packet)
{
int ret;
int8_t frame = -1;

k_sem_take(&_transmit_sem, K_FOREVER);
ret = k_sem_take(&_transmit_sem,
K_SECONDS(HUBBLE_SAT_TRANSMISSION_TIMEOUT_S));
if (ret != 0) {
return ret;
}

k_sem_reset(&_symbol_sem);

_dppi_enable();
Expand All @@ -233,15 +242,18 @@ int hubble_sat_soc_packet_send(const struct hubble_sat_packet_frames *packet)

hubble_nrf_lib_frequency_set(packet->frame[frame].channel,
packet->frame[frame].data[data_pos]);
k_sem_take(&_symbol_sem, K_FOREVER);
ret = k_sem_take(&_symbol_sem, WAIT_SYMBOL_TIMEOUT_US);
if (ret != 0) {
break;
}
}

_dppi_disable();
_timer_disable();

k_sem_give(&_transmit_sem);

return 0;
return ret;
}

#ifdef CONFIG_HUBBLE_SAT_NETWORK_DTM_MODE
Expand Down
19 changes: 14 additions & 5 deletions port/zephyr/boards/silabs/efr32/radio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ static void _timer_cb(sl_rail_handle_t rail_handle)
static int _radio_cw_start(uint8_t channel, uint16_t step, uint32_t delay,
uint32_t duration_us)
{
int ret;
sl_rail_status_t status;
sl_rail_time_t anchor;

Expand All @@ -126,21 +127,24 @@ static int _radio_cw_start(uint8_t channel, uint16_t step, uint32_t delay,
return _sl_status_to_errno(status);
}

k_sem_take(&_symbol_sem, K_FOREVER);
ret = k_sem_take(&_symbol_sem, (2 * duration_us));

status = sl_rail_stop_tx_stream(_rail_handle);
if (status != SL_RAIL_STATUS_NO_ERROR) {
return _sl_status_to_errno(status);
}

if (ret != 0) {
Comment on lines 134 to +137

@HongNguyen635 HongNguyen635 Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should we preserve the error from k_sem_take here?
Maybe:

return (ret != 0) ? ret : _sl_status_to_errno(status);

But I'm not sure if this makes it hard to read / maintain.

return ret;
}

status = sl_rail_set_timer(_rail_handle, anchor + duration_us + delay,
SL_RAIL_TIME_ABSOLUTE, &_timer_cb);
if (status != SL_RAIL_STATUS_NO_ERROR) {
return _sl_status_to_errno(status);
}

k_sem_take(&_symbol_sem, K_FOREVER);
return 0;
return k_sem_take(&_symbol_sem, (2 * delay));
}

static int _radio_channel_set(uint8_t channel)
Expand Down Expand Up @@ -255,10 +259,15 @@ int hubble_sat_board_disable(void)

int hubble_sat_board_packet_send(const struct hubble_sat_packet_frames *packet)
{
int ret = 0;
int ret;
int8_t frame = -1;

k_sem_take(&_transmit_sem, K_FOREVER);
ret = k_sem_take(&_transmit_sem,
K_SECONDS(HUBBLE_SAT_TRANSMISSION_TIMEOUT_S));
if (ret != 0) {
return ret;
}

k_sem_reset(&_symbol_sem);

for (uint8_t i = 0; i < packet->total_number_of_symbols; i++) {
Expand Down
Loading