[M] Change in libosmocore[master]: soft_uart: fix pulling a small number of Tx bits

2023-11-21 Thread laforge
laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email )

Change subject: soft_uart: fix pulling a small number of Tx bits
..

soft_uart: fix pulling a small number of Tx bits

Change-Id: I454c8786697a6f2389d56b350e6e20ca953fe859
Related: OS#4396
---
M src/core/soft_uart.c
M tests/soft_uart/soft_uart_test.ok
2 files changed, 50 insertions(+), 73 deletions(-)

Approvals:
  Jenkins Builder: Verified
  jolly: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved




diff --git a/src/core/soft_uart.c b/src/core/soft_uart.c
index c94070f..e0f0780 100644
--- a/src/core/soft_uart.c
+++ b/src/core/soft_uart.c
@@ -217,7 +217,7 @@

switch (suart->tx.flow_state) {
case SUART_FLOW_ST_IDLE:
-   if (msgb_length(msg) > 0) { /* if we have pending data */
+   if (msg && msgb_length(msg) > 0) { /* if we have pending data */
suart->tx.shift_reg = msgb_pull_u8(msg);
suart->tx.flow_state = SUART_FLOW_ST_DATA;
suart->tx.bit_count = 0;
@@ -283,21 +283,31 @@
 int osmo_soft_uart_tx_ubits(struct osmo_soft_uart *suart, ubit_t *ubits, 
size_t n_ubits)
 {
const struct osmo_soft_uart_cfg *cfg = >cfg;
-   size_t n_frame_bits;
-   struct msgb *msg;
+   size_t n_frame_bits, n_chars;
+   struct msgb *msg = NULL;

/* calculate UART frame size for the effective config */
n_frame_bits = 1 + cfg->num_data_bits + cfg->num_stop_bits;
if (cfg->parity_mode != OSMO_SUART_PARITY_NONE)
n_frame_bits += 1;

-   /* allocate a Tx buffer msgb */
-   msg = msgb_alloc_c(suart, n_ubits / n_frame_bits, "soft_uart_tx");
-   OSMO_ASSERT(msg != NULL);
+   /* calculate the number of characters we can fit into n_ubits */
+   n_chars = n_ubits / n_frame_bits;
+   if (n_chars == 0) {
+   /* we can transmit at least one character */
+   if (suart->tx.flow_state == SUART_FLOW_ST_IDLE)
+   n_chars = 1;
+   }

-   /* call the .tx_cb() to populate the Tx buffer */
-   OSMO_ASSERT(cfg->tx_cb != NULL);
-   suart->cfg.tx_cb(cfg->priv, msg);
+   if (n_chars > 0) {
+   /* allocate a Tx buffer msgb */
+   msg = msgb_alloc_c(suart, n_chars, "soft_uart_tx");
+   OSMO_ASSERT(msg != NULL);
+
+   /* call the .tx_cb() to populate the Tx buffer */
+   OSMO_ASSERT(cfg->tx_cb != NULL);
+   suart->cfg.tx_cb(cfg->priv, msg);
+   }
 
for (size_t i = 0; i < n_ubits; i++)
ubits[i] = osmo_uart_tx_bit(suart, msg);
diff --git a/tests/soft_uart/soft_uart_test.ok 
b/tests/soft_uart/soft_uart_test.ok
index a160ad3..9ce3bc8 100644
--- a/tests/soft_uart/soft_uart_test.ok
+++ b/tests/soft_uart/soft_uart_test.ok
@@ -172,74 +172,31 @@
 suart_tx_cb(len=0/4):
 test_tx_rx_exec_one(n_bits_total=32):    
  test_tx_rx_pull_n(): pulling 32 bits (1 at a time)
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+01010101010101010101010101010101
  test_tx_rx_pull_n(): feeding 32 bits into the receiver
+suart_rx_cb(flags=00): 55 55 55
  test_tx_rx_pull_n(): pulling 32 bits (2 at a time)
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+01010101010101010101010101010101
  test_tx_rx_pull_n(): feeding 32 bits into the receiver
+suart_rx_cb(flags=00): 55 55 55
  test_tx_rx_pull_n(): pulling 32 bits (4 at a time)
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):

[M] Change in libosmocore[master]: soft_uart: fix pulling a small number of Tx bits

2023-11-21 Thread laforge
Attention is currently required from: fixeria, jolly.

laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email )

Change subject: soft_uart: fix pulling a small number of Tx bits
..


Patch Set 2: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I454c8786697a6f2389d56b350e6e20ca953fe859
Gerrit-Change-Number: 35076
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: jolly 
Gerrit-Reviewer: laforge 
Gerrit-Attention: jolly 
Gerrit-Attention: fixeria 
Gerrit-Comment-Date: Tue, 21 Nov 2023 16:35:30 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: soft_uart: fix pulling a small number of Tx bits

2023-11-21 Thread fixeria
Attention is currently required from: jolly.

fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email )

Change subject: soft_uart: fix pulling a small number of Tx bits
..


Patch Set 2:

(1 comment)

File src/core/soft_uart.c:

https://gerrit.osmocom.org/c/libosmocore/+/35076/comment/eb99f9e8_f228bc72
PS1, Line 283: osmo_soft_uart_tx_ubits
> Ack, thanks! Will do. I think for `n_ubits == 0` we should just return early.
Done here: https://gerrit.osmocom.org/c/libosmocore/+/35083.



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I454c8786697a6f2389d56b350e6e20ca953fe859
Gerrit-Change-Number: 35076
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: jolly 
Gerrit-Reviewer: laforge 
Gerrit-Attention: jolly 
Gerrit-Comment-Date: Tue, 21 Nov 2023 13:34:36 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: jolly 
Comment-In-Reply-To: fixeria 
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: soft_uart: fix pulling a small number of Tx bits

2023-11-21 Thread laforge
Attention is currently required from: fixeria, jolly.

laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email )

Change subject: soft_uart: fix pulling a small number of Tx bits
..


Patch Set 1: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I454c8786697a6f2389d56b350e6e20ca953fe859
Gerrit-Change-Number: 35076
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: jolly 
Gerrit-Reviewer: laforge 
Gerrit-Attention: jolly 
Gerrit-Attention: fixeria 
Gerrit-Comment-Date: Tue, 21 Nov 2023 11:48:12 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: soft_uart: fix pulling a small number of Tx bits

2023-11-21 Thread fixeria
Attention is currently required from: jolly.

fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email )

Change subject: soft_uart: fix pulling a small number of Tx bits
..


Patch Set 1:

(1 comment)

File src/core/soft_uart.c:

https://gerrit.osmocom.org/c/libosmocore/+/35076/comment/67e603e1_29169a28
PS1, Line 283: osmo_soft_uart_tx_ubits
> Looks good to me. Maybe add a check for n_ubits being 0. […]
Ack, thanks! Will do. I think for `n_ubits == 0` we should just return early.



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I454c8786697a6f2389d56b350e6e20ca953fe859
Gerrit-Change-Number: 35076
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: jolly 
Gerrit-Attention: jolly 
Gerrit-Comment-Date: Tue, 21 Nov 2023 08:18:20 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: jolly 
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: soft_uart: fix pulling a small number of Tx bits

2023-11-21 Thread jolly
Attention is currently required from: fixeria.

jolly has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email )

Change subject: soft_uart: fix pulling a small number of Tx bits
..


Patch Set 1: Code-Review+1

(1 comment)

File src/core/soft_uart.c:

https://gerrit.osmocom.org/c/libosmocore/+/35076/comment/c61037c2_b0f69a10
PS1, Line 283: osmo_soft_uart_tx_ubits
Looks good to me. Maybe add a check for n_ubits being 0. The current version 
would request 1 character and provides no bits.



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I454c8786697a6f2389d56b350e6e20ca953fe859
Gerrit-Change-Number: 35076
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: jolly 
Gerrit-Attention: fixeria 
Gerrit-Comment-Date: Tue, 21 Nov 2023 08:05:02 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: soft_uart: fix pulling a small number of Tx bits

2023-11-20 Thread fixeria
fixeria has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/libosmocore/+/35076?usp=email )


Change subject: soft_uart: fix pulling a small number of Tx bits
..

soft_uart: fix pulling a small number of Tx bits

Change-Id: I454c8786697a6f2389d56b350e6e20ca953fe859
Related: OS#4396
---
M src/core/soft_uart.c
M tests/soft_uart/soft_uart_test.ok
2 files changed, 50 insertions(+), 73 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/76/35076/1

diff --git a/src/core/soft_uart.c b/src/core/soft_uart.c
index c94070f..e0f0780 100644
--- a/src/core/soft_uart.c
+++ b/src/core/soft_uart.c
@@ -217,7 +217,7 @@

switch (suart->tx.flow_state) {
case SUART_FLOW_ST_IDLE:
-   if (msgb_length(msg) > 0) { /* if we have pending data */
+   if (msg && msgb_length(msg) > 0) { /* if we have pending data */
suart->tx.shift_reg = msgb_pull_u8(msg);
suart->tx.flow_state = SUART_FLOW_ST_DATA;
suart->tx.bit_count = 0;
@@ -283,21 +283,31 @@
 int osmo_soft_uart_tx_ubits(struct osmo_soft_uart *suart, ubit_t *ubits, 
size_t n_ubits)
 {
const struct osmo_soft_uart_cfg *cfg = >cfg;
-   size_t n_frame_bits;
-   struct msgb *msg;
+   size_t n_frame_bits, n_chars;
+   struct msgb *msg = NULL;

/* calculate UART frame size for the effective config */
n_frame_bits = 1 + cfg->num_data_bits + cfg->num_stop_bits;
if (cfg->parity_mode != OSMO_SUART_PARITY_NONE)
n_frame_bits += 1;

-   /* allocate a Tx buffer msgb */
-   msg = msgb_alloc_c(suart, n_ubits / n_frame_bits, "soft_uart_tx");
-   OSMO_ASSERT(msg != NULL);
+   /* calculate the number of characters we can fit into n_ubits */
+   n_chars = n_ubits / n_frame_bits;
+   if (n_chars == 0) {
+   /* we can transmit at least one character */
+   if (suart->tx.flow_state == SUART_FLOW_ST_IDLE)
+   n_chars = 1;
+   }

-   /* call the .tx_cb() to populate the Tx buffer */
-   OSMO_ASSERT(cfg->tx_cb != NULL);
-   suart->cfg.tx_cb(cfg->priv, msg);
+   if (n_chars > 0) {
+   /* allocate a Tx buffer msgb */
+   msg = msgb_alloc_c(suart, n_chars, "soft_uart_tx");
+   OSMO_ASSERT(msg != NULL);
+
+   /* call the .tx_cb() to populate the Tx buffer */
+   OSMO_ASSERT(cfg->tx_cb != NULL);
+   suart->cfg.tx_cb(cfg->priv, msg);
+   }

for (size_t i = 0; i < n_ubits; i++)
ubits[i] = osmo_uart_tx_bit(suart, msg);
diff --git a/tests/soft_uart/soft_uart_test.ok 
b/tests/soft_uart/soft_uart_test.ok
index a160ad3..9ce3bc8 100644
--- a/tests/soft_uart/soft_uart_test.ok
+++ b/tests/soft_uart/soft_uart_test.ok
@@ -172,74 +172,31 @@
 suart_tx_cb(len=0/4):
 test_tx_rx_exec_one(n_bits_total=32):    
  test_tx_rx_pull_n(): pulling 32 bits (1 at a time)
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+01010101010101010101010101010101
  test_tx_rx_pull_n(): feeding 32 bits into the receiver
+suart_rx_cb(flags=00): 55 55 55
  test_tx_rx_pull_n(): pulling 32 bits (2 at a time)
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+suart_tx_cb(len=1/1): 55
+01010101010101010101010101010101
  test_tx_rx_pull_n(): feeding 32 bits into the receiver
+suart_rx_cb(flags=00): 55 55 55
  test_tx_rx_pull_n(): pulling 32 bits (4 at a time)
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):
-suart_tx_cb(len=0/0):