Skip to content

Commit

Permalink
Merge pull request #29 from hideakitai/fix/transaction-timeout
Browse files Browse the repository at this point in the history
fix: transaction timeout
  • Loading branch information
hideakitai committed Feb 12, 2024
2 parents 10eb7f9 + 98631a6 commit 2ea5400
Show file tree
Hide file tree
Showing 11 changed files with 286 additions and 31 deletions.
74 changes: 53 additions & 21 deletions ESP32SPISlave.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ static constexpr const char *TAG = "ESP32SPISlave";
static constexpr int SPI_SLAVE_TASK_STASCK_SIZE = 1024 * 2;
static constexpr int SPI_SLAVE_TASK_PRIORITY = 5;

static constexpr int SLAVE_QUEUE_TRANS_TIMEOUT_TICKS = pdMS_TO_TICKS(5000);
static constexpr int SLAVE_GET_TRANS_RESULT_TIMEOUT_TICKS = portMAX_DELAY;

static QueueHandle_t s_trans_queue_handle {NULL};
static constexpr int SEND_TRANS_QUEUE_TIMEOUT_TICKS = pdMS_TO_TICKS(5000);
static constexpr int RECV_TRANS_QUEUE_TIMEOUT_TICKS = portMAX_DELAY;
Expand Down Expand Up @@ -75,6 +72,7 @@ struct spi_transaction_context_t
{
spi_slave_transaction_t *trans;
size_t size;
TickType_t timeout_ticks;
};

struct spi_slave_cb_user_context_t
Expand Down Expand Up @@ -134,25 +132,32 @@ void spi_slave_task(void *arg)

// execute new transaction if transaction request received from main task
ESP_LOGD(TAG, "new transaction request received (size = %u)", trans_ctx.size);
std::vector<esp_err_t> errs;
errs.reserve(trans_ctx.size);
for (size_t i = 0; i < trans_ctx.size; ++i) {
spi_slave_transaction_t *trans = &trans_ctx.trans[i];
esp_err_t err = spi_slave_queue_trans(ctx->host, trans, SLAVE_QUEUE_TRANS_TIMEOUT_TICKS);
esp_err_t err = spi_slave_queue_trans(ctx->host, trans, trans_ctx.timeout_ticks);
if (err != ESP_OK) {
ESP_LOGE(TAG, "failed to execute spi_slave_queue_trans(): %d", err);
ESP_LOGE(TAG, "failed to execute spi_slave_queue_trans(): 0x%X", err);
}
errs.push_back(err);
}

// wait for the completion of all of the queued transactions
for (size_t i = 0; i < trans_ctx.size; ++i) {
// wait for completion of next transaction
size_t num_received_bytes = 0;
spi_slave_transaction_t *rtrans;
esp_err_t err = spi_slave_get_trans_result(ctx->host, &rtrans, SLAVE_GET_TRANS_RESULT_TIMEOUT_TICKS);
if (err != ESP_OK) {
ESP_LOGE(TAG, "failed to execute spi_device_get_trans_result(): %d", err);
if (errs[i] == ESP_OK) {
spi_slave_transaction_t *rtrans;
esp_err_t err = spi_slave_get_trans_result(ctx->host, &rtrans, trans_ctx.timeout_ticks);
if (err != ESP_OK) {
ESP_LOGE(TAG, "failed to execute spi_slave_get_trans_result(): 0x%X", err);
} else {
num_received_bytes = rtrans->trans_len / 8; // bit -> byte
ESP_LOGD(TAG, "transaction complete: %d bits (%d bytes) received", rtrans->trans_len, num_received_bytes);
}
} else {
num_received_bytes = rtrans->trans_len / 8; // bit -> byte
ESP_LOGD(TAG, "transaction complete: %d bits (%d bytes) received", rtrans->trans_len, num_received_bytes);
ESP_LOGE(TAG, "skip spi_slave_get_trans_result() because queue was failed: index = %u", i);
}

// send the received bytes back to main task
Expand Down Expand Up @@ -285,6 +290,7 @@ class Slave
/// @param tx_buf pointer to the buffer of data to be sent
/// @param rx_buf pointer to the buffer of data to be received
/// @param size size of data to be sent
/// @param timeout_ms timeout in milliseconds
/// @return the size of received bytes
/// @note this function is blocking until the completion of transmission
size_t transfer(const uint8_t* tx_buf, uint8_t* rx_buf, size_t size, uint32_t timeout_ms = 0)
Expand All @@ -296,6 +302,7 @@ class Slave
/// @param tx_buf pointer to the buffer of data to be sent
/// @param rx_buf pointer to the buffer of data to be received
/// @param size size of data to be sent
/// @param timeout_ms timeout in milliseconds
/// @return the size of received bytes
/// @note this function is blocking until the completion of transmission
size_t transfer(
Expand Down Expand Up @@ -363,21 +370,33 @@ class Slave
/// @return a vector of the received bytes for all transactions
std::vector<size_t> wait(uint32_t timeout_ms = 0)
{
if (!this->trigger()) {
size_t num_will_be_queued = this->transactions.size();
if (!this->trigger(timeout_ms)) {
return std::vector<size_t>();
}
return this->waitTransaction(timeout_ms);
return this->waitTransaction(num_will_be_queued);
}

/// @brief execute queued transactions asynchronously in the background (without blocking)
/// numBytesReceivedAll() or numBytesReceived() is required to confirm the results of transactions
/// rx_buf is automatically updated after the completion of each transaction.
/// @param timeout_ms timeout in milliseconds
/// @return true if the transaction is queued successfully, false otherwise
bool trigger()
bool trigger(uint32_t timeout_ms = 0)
{
if (this->transactions.empty()) {
ESP_LOGW(TAG, "failed to trigger transaction: no transaction is queued");
return false;
}
if (this->numTransactionsInFlight() > 0) {
ESP_LOGW(TAG, "failed to trigger transaction: there are already in-flight transactions");
return false;
}

spi_transaction_context_t trans_ctx {
.trans = new spi_slave_transaction_t[this->transactions.size()],
.size = this->transactions.size(),
.timeout_ticks = timeout_ms == 0 ? portMAX_DELAY : pdMS_TO_TICKS(timeout_ms),
};
for (size_t i = 0; i < this->transactions.size(); i++) {
trans_ctx.trans[i] = std::move(this->transactions[i]);
Expand Down Expand Up @@ -440,6 +459,21 @@ class Slave
return results;
}

/// @brief check if the queued transactions are completed and all results are handled
/// @return true if the queued transactions are completed and all results are handled, false otherwise
bool hasTransactionsCompletedAndAllResultsHandled()
{
return this->numTransactionsInFlight() == 0 && this->numTransactionsCompleted() == 0;
}

/// @brief check if the queued transactions are completed
/// @param num_queued the number of queued transactions
/// @return true if the queued transactions are completed, false otherwise
bool hasTransactionsCompletedAndAllResultsReady(size_t num_queued)
{
return this->numTransactionsInFlight() == 0 && this->numTransactionsCompleted() == num_queued;
}

// ===== Main Configurations =====
// set these optional parameters before begin() if you want

Expand Down Expand Up @@ -563,15 +597,13 @@ class Slave
this->transactions.push_back(std::move(trans));
}

std::vector<size_t> waitTransaction(uint32_t timeout_ms)
std::vector<size_t> waitTransaction(size_t num_will_be_queued)
{
uint32_t start_ms = millis();
while ((timeout_ms == 0) ? true : (millis() < start_ms + timeout_ms)) {
if (this->numTransactionsInFlight() == 0) {
return this->numBytesReceivedAll();
}
// transactions inside of spi task will be timeout if failed in the background
while (!this->hasTransactionsCompletedAndAllResultsReady(num_will_be_queued)) {
vTaskDelay(1);
}
return std::vector<size_t>();
return this->numBytesReceivedAll();
}
};

Expand Down
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ You can use Master and Slave almost the same way (omit the Slave example here).
void loop()
{
// if no transaction is in flight and all results are handled, queue new transactions
if (slave.numTransactionsInFlight() == 0 && slave.numTransactionsCompleted() == 0) {
if (slave.hasTransactionsCompletedAndAllResultsHandled()) {
// do some initialization for tx_buf and rx_buf

// queue multiple transactions
Expand All @@ -121,7 +121,7 @@ void loop()
// NOTE: you can't touch dma_tx/rx_buf because it's in-flight in the background

// if all transactions are completed and all results are ready, handle results
if (slave.numTransactionsInFlight() == 0 && slave.numTransactionsCompleted() == QUEUE_SIZE) {
if (slave.hasTransactionsCompletedAndAllResultsReady(QUEUE_SIZE)) {
// get received bytes for all transactions
const std::vector<size_t> received_bytes = slave.numBytesReceivedAll();

Expand Down Expand Up @@ -213,6 +213,10 @@ size_t numTransactionsCompleted();
size_t numBytesReceived();
/// @brief return all results of the completed transactions (received bytes)
std::vector<size_t> numBytesReceivedAll();
/// @brief check if the queued transactions are completed and all results are handled
bool hasTransactionsCompletedAndAllResultsHandled();
/// @brief check if the queued transactions are completed
bool hasTransactionsCompletedAndAllResultsReady(size_t num_queued);

// ===== Main Configurations =====
// set these optional parameters before begin() if you want
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ void setup()
void loop()
{
// if no transaction is in flight and all results are handled, queue new transactions
if (slave.numTransactionsInFlight() == 0 && slave.numTransactionsCompleted() == 0) {
if (slave.hasTransactionsCompletedAndAllResultsHandled()) {
// initialize tx/rx buffers
Serial.println("initialize tx/rx buffers");
initializeBuffers(tx_buf, rx_buf, BUFFER_SIZE, 0);
Expand All @@ -48,7 +48,7 @@ void loop()
// NOTE: you can't touch dma_tx/rx_buf because it's in-flight in the background

// if all transactions are completed and all results are ready, handle results
if (slave.numTransactionsInFlight() == 0 && slave.numTransactionsCompleted() == QUEUE_SIZE) {
if (slave.hasTransactionsCompletedAndAllResultsReady(QUEUE_SIZE)) {
// process received data from slave
Serial.println("all queued transactions completed. start verifying received data from slave");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ void setup()
void loop()
{
// if no transaction is in flight and all results are handled, queue new transactions
if (slave.numTransactionsInFlight() == 0 && slave.numTransactionsCompleted() == 0) {
if (slave.hasTransactionsCompletedAndAllResultsHandled()) {
// initialize tx/rx buffers
Serial.println("initialize tx/rx buffers");
initializeBuffers(tx_buf, rx_buf, BUFFER_SIZE, 0);
Expand All @@ -43,7 +43,7 @@ void loop()
// NOTE: you can't touch dma_tx/rx_buf because it's in-flight in the background

// if all transactions are completed and all results are ready, handle results
if (slave.numTransactionsInFlight() == 0 && slave.numTransactionsCompleted() == QUEUE_SIZE) {
if (slave.hasTransactionsCompletedAndAllResultsReady(QUEUE_SIZE)) {
// process received data from slave
Serial.println("all queued transactions completed. start verifying received data from slave");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void loop()
}

// if no transaction is in flight and all results are handled, queue new transactions
if (slave.numTransactionsInFlight() == 0 && slave.numTransactionsCompleted() == 0) {
if (slave.hasTransactionsCompletedAndAllResultsHandled()) {
// initialize tx/rx buffers
Serial.println("initialize tx/rx buffers");
initializeBuffers(tx_buf, rx_buf, BUFFER_SIZE, 0);
Expand All @@ -76,7 +76,7 @@ void loop()
// NOTE: you can't touch dma_tx/rx_buf because it's in-flight in the background

// if all transactions are completed and all results are ready, handle results
if (slave.numTransactionsInFlight() == 0 && slave.numTransactionsCompleted() == QUEUE_SIZE) {
if (slave.hasTransactionsCompletedAndAllResultsReady(QUEUE_SIZE)) {
// process received data from slave
Serial.println("all queued transactions completed. start verifying received data from slave");

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#pragma once

#include <Arduino.h>
#include <cstdint>
#include <cstddef>

void dumpBuffers(const char *title, const uint8_t *buf, size_t start, size_t len)
{
// show title and range
if (len == 1)
printf("%s [%d]: ", title, start);
else
printf("%s [%d-%d]: ", title, start, start + len - 1);

// show data in the range
for (size_t i = 0; i < len; i++) {
printf("%02X ", buf[start + i]);
}
printf("\n");
}

bool verifyAndDumpDifference(const char *a_title, const uint8_t *a_buf, size_t a_size, const char *b_title, const uint8_t *b_buf, size_t b_size)
{
bool verified = true;

if (a_size != b_size) {
printf("received data size does not match: expected = %d / actual = %d\n", a_size, b_size);
return false;
}

for (size_t i = 0; i < a_size; i++) {
// if a_buf and b_buf is same, continue
if (a_buf[i] == b_buf[i]) {
continue;
}

verified = false;

// if a_buf[i] and b_buf[i] is not same, check the range that has difference
size_t j = 1;
while (a_buf[i + j] != b_buf[i + j]) {
j++;
}

// dump different data range
dumpBuffers(a_title, a_buf, i, j);
dumpBuffers(b_title, b_buf, i, j);

// restart from next same index (-1 considers i++ in for())
i += j - 1;
}
return verified;
}

void initializeBuffers(uint8_t *tx, uint8_t *rx, size_t size, size_t offset = 0)
{
if (tx) {
for (size_t i = 0; i < size; i++) {
tx[i] = (i + offset) & 0xFF;
}
}
if (rx) {
memset(rx, 0, size);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#include <SPI.h>
#include "helper.h"

SPIClass master(HSPI);
#ifdef CONFIG_IDF_TARGET_ESP32
static constexpr uint8_t PIN_SS = 15;
#else
static constexpr uint8_t PIN_SS = SS;
#endif

static constexpr size_t BUFFER_SIZE = 8;
uint8_t tx_buf[BUFFER_SIZE] {1, 2, 3, 4, 5, 6, 7, 8};
uint8_t rx_buf[BUFFER_SIZE] {0, 0, 0, 0, 0, 0, 0, 0};

void setup()
{
Serial.begin(115200);

delay(2000);

pinMode(PIN_SS, OUTPUT);
digitalWrite(PIN_SS, HIGH);
master.begin(SCK, MISO, MOSI, PIN_SS);

delay(2000);

Serial.println("start spi master");
}

void loop()
{
// initialize tx/rx buffers
initializeBuffers(tx_buf, rx_buf, BUFFER_SIZE);

master.beginTransaction(SPISettings(1000000, MSBFIRST, SPI_MODE0));
digitalWrite(PIN_SS, LOW);
master.transferBytes(tx_buf, rx_buf, BUFFER_SIZE);
digitalWrite(PIN_SS, HIGH);
master.endTransaction();

// verify and dump difference with received data
if (verifyAndDumpDifference("master", tx_buf, BUFFER_SIZE, "slave", rx_buf, BUFFER_SIZE)) {
Serial.println("successfully received expected data from slave");
} else {
Serial.println("unexpected difference found between master/slave data");
}

delay(2500); // slave will timeout after 1000ms
}
Loading

0 comments on commit 2ea5400

Please sign in to comment.