Skip to content

Commit 887edc7

Browse files
zfieldszfields
authored
fix: I2C mutex scheme for transactions (#211)
1 parent 56d35ef commit 887edc7

File tree

4 files changed

+63
-66
lines changed

4 files changed

+63
-66
lines changed

‎n_request.c‎

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ void _noteSuspendTransactionDebug(void)
116116
*/
117117
NOTE_C_STATIC uint32_t _noteTransaction_calculateTimeoutMs(J *req, bool isReq)
118118
{
119-
uint32_t result = (CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000);
119+
uint32_t result = ((CARD_INTER_TRANSACTION_TIMEOUT_SEC - 1) * 1000);
120120

121121
// Interrogate the request
122122
if (JContainsString(req, (isReq ? "req" : "cmd"), "note.add")) {
@@ -137,13 +137,13 @@ NOTE_C_STATIC uint32_t _noteTransaction_calculateTimeoutMs(J *req, bool isReq)
137137
} else if (JIsPresent(req, "seconds")) {
138138
NOTE_C_LOG_DEBUG("Using `seconds` parameter value for timeout.");
139139
result = (JGetInt(req, "seconds") * 1000);
140-
} else {
141-
NOTE_C_LOG_DEBUG("No `milliseconds` or `seconds` parameter "
142-
"provided. Defaulting to 90-second timeout.");
143-
result = (90 * 1000);
144140
}
145141
}
146142

143+
// Add one second to the timeout, to provide time for the Notecard to
144+
// timeout first, then report the timeout to the host, when applicable.
145+
result += 1000;
146+
147147
return result;
148148
}
149149

@@ -436,9 +436,6 @@ J *_noteTransactionShouldLock(J *req, bool lockNotecard)
436436
// Serialize the JSON request
437437
char *json = JPrintUnformatted(req); // `json` allocated, must be freed
438438
if (json == NULL) {
439-
if (lockNotecard) {
440-
_UnlockNote();
441-
}
442439
_TransactionStop();
443440
NOTE_C_LOG_ERROR(ERRSTR("failed to serialize JSON request", c_mem));
444441
return NULL;
@@ -492,28 +489,6 @@ J *_noteTransactionShouldLock(J *req, bool lockNotecard)
492489
}
493490
#endif
494491

495-
// If a reset of the I/O interface is required for any reason, do it now.
496-
// We must do this before acquiring lock.
497-
if (resetRequired) {
498-
NOTE_C_LOG_DEBUG("Resetting Notecard I/O Interface...");
499-
if (!NoteReset()) {
500-
_Free(json);
501-
_TransactionStop();
502-
const char *errStr = ERRSTR("failed to reset Notecard interface {io}", c_iobad);
503-
if (cmdFound) {
504-
NOTE_C_LOG_ERROR(errStr);
505-
return NULL;
506-
}
507-
return _errDoc(id, errStr);
508-
}
509-
}
510-
511-
// Take the lock on the Notecard. This is required to ensure that we don't
512-
// have multiple threads trying to access the Notecard at the same time.
513-
if (lockNotecard) {
514-
_LockNote();
515-
}
516-
517492
// Calculate the transaction timeout based on the parameters in the request.
518493
const uint32_t transactionTimeoutMs = _noteTransaction_calculateTimeoutMs(req, reqFound);
519494

@@ -547,6 +522,30 @@ J *_noteTransactionShouldLock(J *req, bool lockNotecard)
547522
}
548523
#endif // !NOTE_C_LOW_MEM
549524

525+
// Take the lock on the Notecard. This is required to ensure that we don't
526+
// have multiple threads trying to access the Notecard at the same time.
527+
if (lockNotecard) {
528+
_LockNote();
529+
}
530+
531+
// If a reset of the I/O interface is required for any reason, do it now.
532+
if (resetRequired) {
533+
NOTE_C_LOG_DEBUG("Resetting Notecard I/O Interface...");
534+
if ((resetRequired = !_Reset())) {
535+
if (lockNotecard) {
536+
_UnlockNote();
537+
}
538+
_Free(json);
539+
_TransactionStop();
540+
const char *errStr = ERRSTR("failed to reset Notecard interface {io}", c_iobad);
541+
if (cmdFound) {
542+
NOTE_C_LOG_ERROR(errStr);
543+
return NULL;
544+
}
545+
return _errDoc(id, errStr);
546+
}
547+
}
548+
550549
// If we're performing retries, this is where we come back to
551550
// after a failed transaction.
552551
const char *errStr = NULL;

‎note.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ enum {
4141

4242
#define NOTE_C_VERSION_MAJOR 2
4343
#define NOTE_C_VERSION_MINOR 5
44-
#define NOTE_C_VERSION_PATCH 2
44+
#define NOTE_C_VERSION_PATCH 3
4545

4646
#define NOTE_C_VERSION NOTE_C_STRINGIZE(NOTE_C_VERSION_MAJOR) "." NOTE_C_STRINGIZE(NOTE_C_VERSION_MINOR) "." NOTE_C_STRINGIZE(NOTE_C_VERSION_PATCH)
4747

‎test/src/NoteTransaction_test.cpp‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ FAKE_VALUE_FUNC(const char *, _noteJSONTransaction, const char *, size_t, char *
2626
FAKE_VALUE_FUNC(bool, _noteTransactionStart, uint32_t)
2727
FAKE_VOID_FUNC(NoteDebugWithLevel, uint8_t, const char *)
2828
FAKE_VOID_FUNC(NoteDelayMs, uint32_t)
29-
FAKE_VALUE_FUNC(bool, NoteReset)
3029
FAKE_VALUE_FUNC(J *, NoteUserAgent)
3130

3231
namespace
@@ -180,7 +179,7 @@ SCENARIO("NoteTransaction")
180179

181180
// NoteReset's mock should succeed unless the test explicitly instructs
182181
// it to fail.
183-
NoteReset_fake.return_val = true;
182+
_noteHardReset_fake.return_val = true;
184183
_noteTransactionStart_fake.return_val = true;
185184
_crcError_fake.return_val = false;
186185

@@ -511,10 +510,10 @@ SCENARIO("NoteTransaction")
511510
REQUIRE(req != NULL);
512511
NoteResetRequired();
513512
// Force NoteReset failure.
514-
NoteReset_fake.return_val = false;
513+
_noteHardReset_fake.return_val = false;
515514

516515
J *resp = NoteTransaction(req);
517-
REQUIRE(NoteReset_fake.call_count == 1);
516+
REQUIRE(_noteHardReset_fake.call_count == 1);
518517

519518
// The transaction shouldn't be attempted if reset failed.
520519
CHECK(_noteJSONTransaction_fake.call_count == 0);
@@ -530,10 +529,10 @@ SCENARIO("NoteTransaction")
530529
REQUIRE(req != NULL);
531530
NoteResetRequired();
532531
// Force NoteReset failure.
533-
NoteReset_fake.return_val = false;
532+
_noteHardReset_fake.return_val = false;
534533

535534
J *resp = NoteTransaction(req);
536-
REQUIRE(NoteReset_fake.call_count == 1);
535+
REQUIRE(_noteHardReset_fake.call_count == 1);
537536

538537
// The transaction shouldn't be attempted if reset failed.
539538
CHECK(_noteJSONTransaction_fake.call_count == 0);
@@ -994,7 +993,6 @@ SCENARIO("NoteTransaction")
994993
RESET_FAKE(_noteTransactionStart);
995994
RESET_FAKE(NoteDebugWithLevel);
996995
RESET_FAKE(NoteDelayMs);
997-
RESET_FAKE(NoteReset);
998996
RESET_FAKE(NoteUserAgent);
999997
}
1000998

‎test/src/_noteTransaction_calculateTimeoutMs_test.cpp‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
8282
WHEN("NoteTransaction is called") {
8383
J *resp = NoteTransaction(req);
8484

85-
THEN("The timeout value is set to `seconds * 1000`") {
86-
CHECK(_noteJSONTransaction_fake.arg3_val == (seconds * 1000));
85+
THEN("The timeout value is set to `(seconds + 1) * 1000`") {
86+
CHECK(_noteJSONTransaction_fake.arg3_val == ((seconds + 1) * 1000));
8787
}
8888

8989
JDelete(resp);
@@ -103,8 +103,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
103103
WHEN("NoteTransaction is called") {
104104
J *resp = NoteTransaction(req);
105105

106-
THEN("The timeout value is set to `seconds * 1000`") {
107-
CHECK(_noteJSONTransaction_fake.arg3_val == (seconds * 1000));
106+
THEN("The timeout value is set to `(seconds + 1) * 1000`") {
107+
CHECK(_noteJSONTransaction_fake.arg3_val == ((seconds + 1) * 1000));
108108
}
109109

110110
JDelete(resp);
@@ -124,8 +124,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
124124
WHEN("NoteTransaction is called") {
125125
J *resp = NoteTransaction(req);
126126

127-
THEN("The timeout value is set to `milliseconds`") {
128-
CHECK(_noteJSONTransaction_fake.arg3_val == milliseconds);
127+
THEN("The timeout value is set to `(milliseconds + 1000)`") {
128+
CHECK(_noteJSONTransaction_fake.arg3_val == (milliseconds + 1000));
129129
}
130130

131131
JDelete(resp);
@@ -145,8 +145,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
145145
WHEN("NoteTransaction is called") {
146146
J *resp = NoteTransaction(req);
147147

148-
THEN("The timeout value is set to `milliseconds`") {
149-
CHECK(_noteJSONTransaction_fake.arg3_val == milliseconds);
148+
THEN("The timeout value is set to `(milliseconds + 1000)`") {
149+
CHECK(_noteJSONTransaction_fake.arg3_val == (milliseconds + 1000));
150150
}
151151

152152
JDelete(resp);
@@ -169,8 +169,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
169169
WHEN("NoteTransaction is called") {
170170
J *resp = NoteTransaction(req);
171171

172-
THEN("The timeout value is set to `milliseconds`") {
173-
CHECK(_noteJSONTransaction_fake.arg3_val == milliseconds);
172+
THEN("The timeout value is set to `(milliseconds + 1000)`") {
173+
CHECK(_noteJSONTransaction_fake.arg3_val == (milliseconds + 1000));
174174
}
175175

176176
JDelete(resp);
@@ -193,8 +193,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
193193
WHEN("NoteTransaction is called") {
194194
J *resp = NoteTransaction(req);
195195

196-
THEN("The timeout value is set to `milliseconds`") {
197-
CHECK(_noteJSONTransaction_fake.arg3_val == milliseconds);
196+
THEN("The timeout value is set to `(milliseconds + 1000)`") {
197+
CHECK(_noteJSONTransaction_fake.arg3_val == (milliseconds + 1000));
198198
}
199199

200200
JDelete(resp);
@@ -248,8 +248,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
248248
WHEN("NoteTransaction is called") {
249249
J *resp = NoteTransaction(req);
250250

251-
THEN("The timeout value is set to `seconds * 1000`") {
252-
CHECK(_noteJSONTransaction_fake.arg3_val == (seconds * 1000));
251+
THEN("The timeout value is set to `(seconds + 1) * 1000`") {
252+
CHECK(_noteJSONTransaction_fake.arg3_val == ((seconds + 1) * 1000));
253253
}
254254

255255
JDelete(resp);
@@ -269,8 +269,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
269269
WHEN("NoteTransaction is called") {
270270
J *resp = NoteTransaction(req);
271271

272-
THEN("The timeout value is set to `seconds * 1000`") {
273-
CHECK(_noteJSONTransaction_fake.arg3_val == (seconds * 1000));
272+
THEN("The timeout value is set to `(seconds + 1) * 1000`") {
273+
CHECK(_noteJSONTransaction_fake.arg3_val == ((seconds + 1) * 1000));
274274
}
275275

276276
JDelete(resp);
@@ -290,8 +290,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
290290
WHEN("NoteTransaction is called") {
291291
J *resp = NoteTransaction(req);
292292

293-
THEN("The timeout value is set to `milliseconds`") {
294-
CHECK(_noteJSONTransaction_fake.arg3_val == milliseconds);
293+
THEN("The timeout value is set to `(milliseconds + 1000)`") {
294+
CHECK(_noteJSONTransaction_fake.arg3_val == (milliseconds + 1000));
295295
}
296296

297297
JDelete(resp);
@@ -311,8 +311,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
311311
WHEN("NoteTransaction is called") {
312312
J *resp = NoteTransaction(req);
313313

314-
THEN("The timeout value is set to `milliseconds`") {
315-
CHECK(_noteJSONTransaction_fake.arg3_val == milliseconds);
314+
THEN("The timeout value is set to `(milliseconds + 1000)`") {
315+
CHECK(_noteJSONTransaction_fake.arg3_val == (milliseconds + 1000));
316316
}
317317

318318
JDelete(resp);
@@ -335,8 +335,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
335335
WHEN("NoteTransaction is called") {
336336
J *resp = NoteTransaction(req);
337337

338-
THEN("The timeout value is set to `milliseconds`") {
339-
CHECK(_noteJSONTransaction_fake.arg3_val == milliseconds);
338+
THEN("The timeout value is set to `(milliseconds + 1000)`") {
339+
CHECK(_noteJSONTransaction_fake.arg3_val == (milliseconds + 1000));
340340
}
341341

342342
JDelete(resp);
@@ -359,8 +359,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
359359
WHEN("NoteTransaction is called") {
360360
J *resp = NoteTransaction(req);
361361

362-
THEN("The timeout value is set to `milliseconds`") {
363-
CHECK(_noteJSONTransaction_fake.arg3_val == milliseconds);
362+
THEN("The timeout value is set to `(milliseconds + 1000)`") {
363+
CHECK(_noteJSONTransaction_fake.arg3_val == (milliseconds + 1000));
364364
}
365365

366366
JDelete(resp);
@@ -376,8 +376,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
376376
WHEN("NoteTransaction is called") {
377377
J *resp = NoteTransaction(req);
378378

379-
THEN("The timeout value is set to 90 seconds") {
380-
CHECK(_noteJSONTransaction_fake.arg3_val == (90 * 1000));
379+
THEN("The timeout value is set to `CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000`") {
380+
CHECK(_noteJSONTransaction_fake.arg3_val == (CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000));
381381
}
382382

383383
JDelete(resp);
@@ -393,8 +393,8 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
393393
WHEN("NoteTransaction is called") {
394394
J *resp = NoteTransaction(req);
395395

396-
THEN("The timeout value is set to 90 seconds") {
397-
CHECK(_noteJSONTransaction_fake.arg3_val == (90 * 1000));
396+
THEN("The timeout value is set to `CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000`") {
397+
CHECK(_noteJSONTransaction_fake.arg3_val == (CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000));
398398
}
399399

400400
JDelete(resp);

0 commit comments

Comments
 (0)