Skip to content

Commit edd8cb5

Browse files
aentingeraentinger
authored
By locking each IO expander transaction race conditions due to shared access to Wire can be eliminated. (#39)
1 parent 54f1758 commit edd8cb5

File tree

3 files changed

+42
-10
lines changed

3 files changed

+42
-10
lines changed

‎src/Braccio++.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ BraccioClass::BraccioClass()
2424
: serial485{Serial1, 0, 7, 8} /* TX, DE, RE */
2525
, servos{serial485}
2626
, PD_UFP{PD_LOG_LEVEL_VERBOSE}
27-
, _expander{TCA6424A_ADDRESS_ADDR_HIGH}
27+
, _expander{TCA6424A_ADDRESS_ADDR_HIGH, i2c_mutex}
2828
, _is_ping_allowed{true}
2929
, _is_motor_connected{false}
3030
, _motors_connected_mtx{}
@@ -72,7 +72,7 @@ bool BraccioClass::begin(voidFuncPtr custom_menu)
7272
_bl.turnOn();
7373

7474
SPI.begin();
75-
i2c_mutex.lock();
75+
7676
int ret = _expander.testConnection();
7777

7878
if (ret == false) {
@@ -99,7 +99,6 @@ bool BraccioClass::begin(voidFuncPtr custom_menu)
9999
for (int id = SmartServoClass::MIN_MOTOR_ID; id <= SmartServoClass::MAX_MOTOR_ID; id++) {
100100
setRed(id);
101101
}
102-
i2c_mutex.unlock();
103102

104103
pinMode(BTN_LEFT, INPUT_PULLUP);
105104
pinMode(BTN_RIGHT, INPUT_PULLUP);
@@ -360,14 +359,12 @@ void BraccioClass::motorConnectedThreadFunc()
360359
setMotorConnectionStatus(id, is_connected);
361360
}
362361

363-
i2c_mutex.lock();
364362
for (int id = SmartServoClass::MIN_MOTOR_ID; id <= SmartServoClass::MAX_MOTOR_ID; id++) {
365363
if (connected(id))
366364
setGreen(id);
367365
else
368366
setRed(id);
369367
}
370-
i2c_mutex.unlock();
371368
}
372369
delay(1000);
373370
}

‎src/lib/ioexpander/TCA6424A.cpp

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ THE SOFTWARE.
3535
/** Default constructor, uses default I2C address.
3636
* @see TCA6424A_DEFAULT_ADDRESS
3737
*/
38-
TCA6424A::TCA6424A() {
38+
TCA6424A::TCA6424A(rtos::Mutex & wire_mtx)
39+
: _wire_mtx{wire_mtx}
40+
{
3941
devAddr = TCA6424A_DEFAULT_ADDRESS;
4042
}
4143

@@ -45,8 +47,11 @@ TCA6424A::TCA6424A() {
4547
* @see TCA6424A_ADDRESS_ADDR_LOW
4648
* @see TCA6424A_ADDRESS_ADDR_HIGH
4749
*/
48-
TCA6424A::TCA6424A(uint8_t address) {
49-
devAddr = address;
50+
TCA6424A::TCA6424A(uint8_t address, rtos::Mutex & wire_mtx)
51+
: devAddr{address}
52+
, _wire_mtx{wire_mtx}
53+
{
54+
5055
}
5156

5257
/** Power on and prepare for general usage.
@@ -62,6 +67,7 @@ void TCA6424A::initialize() {
6267
* @return True if connection is valid, false otherwise
6368
*/
6469
bool TCA6424A::testConnection() {
70+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
6571
return I2Cdev::readBytes(devAddr, TCA6424A_RA_INPUT0, 3, buffer) == 3;
6672
}
6773

@@ -71,6 +77,7 @@ bool TCA6424A::testConnection() {
7177
* @return Pin logic level (0 or 1)
7278
*/
7379
bool TCA6424A::readPin(uint16_t pin) {
80+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
7481
I2Cdev::readBit(devAddr, TCA6424A_RA_INPUT0 + (pin / 8), pin % 8, buffer);
7582
return buffer[0];
7683
}
@@ -79,6 +86,7 @@ bool TCA6424A::readPin(uint16_t pin) {
7986
* @return 8 pins' logic levels (0 or 1 for each pin)
8087
*/
8188
uint8_t TCA6424A::readBank(uint8_t bank) {
89+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
8290
I2Cdev::readByte(devAddr, TCA6424A_RA_INPUT0 + bank, buffer);
8391
return buffer[0];
8492
}
@@ -87,6 +95,7 @@ uint8_t TCA6424A::readBank(uint8_t bank) {
8795
* @param banks Container for all bank's pin values (P00-P27)
8896
*/
8997
void TCA6424A::readAll(uint8_t *banks) {
98+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
9099
I2Cdev::readBytes(devAddr, TCA6424A_RA_INPUT0, 3, banks);
91100
}
92101
/** Get all pin logic levels from all banks.
@@ -96,6 +105,7 @@ void TCA6424A::readAll(uint8_t *banks) {
96105
* @param bank2 Container for Bank 2's pin values (P20-P27)
97106
*/
98107
void TCA6424A::readAll(uint8_t *bank0, uint8_t *bank1, uint8_t *bank2) {
108+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
99109
I2Cdev::readBytes(devAddr, TCA6424A_RA_INPUT0, 3, buffer);
100110
*bank0 = buffer[0];
101111
*bank1 = buffer[1];
@@ -110,6 +120,7 @@ void TCA6424A::readAll(uint8_t *bank0, uint8_t *bank1, uint8_t *bank2) {
110120
* @return Pin output setting (0 or 1)
111121
*/
112122
bool TCA6424A::getPinOutputLevel(uint16_t pin) {
123+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
113124
I2Cdev::readBit(devAddr, TCA6424A_RA_OUTPUT0 + (pin / 8), pin % 8, buffer);
114125
return buffer[0];
115126
}
@@ -120,6 +131,7 @@ bool TCA6424A::getPinOutputLevel(uint16_t pin) {
120131
* @return 8 pins' output settings (0 or 1 for each pin)
121132
*/
122133
uint8_t TCA6424A::getBankOutputLevel(uint8_t bank) {
134+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
123135
I2Cdev::readByte(devAddr, TCA6424A_RA_OUTPUT0 + bank, buffer);
124136
return buffer[0];
125137
}
@@ -128,6 +140,7 @@ uint8_t TCA6424A::getBankOutputLevel(uint8_t bank) {
128140
* @param banks Container for all bank's pin values (P00-P27)
129141
*/
130142
void TCA6424A::getAllOutputLevel(uint8_t *banks) {
143+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
131144
I2Cdev::readBytes(devAddr, TCA6424A_RA_OUTPUT0, 3, banks);
132145
}
133146
/** Get all pin output settings from all banks.
@@ -139,6 +152,7 @@ void TCA6424A::getAllOutputLevel(uint8_t *banks) {
139152
* @param bank2 Container for Bank 2's pin values (P20-P27)
140153
*/
141154
void TCA6424A::getAllOutputLevel(uint8_t *bank0, uint8_t *bank1, uint8_t *bank2) {
155+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
142156
I2Cdev::readBytes(devAddr, TCA6424A_RA_OUTPUT0, 3, buffer);
143157
*bank0 = buffer[0];
144158
*bank1 = buffer[1];
@@ -149,19 +163,22 @@ void TCA6424A::getAllOutputLevel(uint8_t *bank0, uint8_t *bank1, uint8_t *bank2)
149163
* @param value New pin output logic level (0 or 1)
150164
*/
151165
void TCA6424A::writePin(uint16_t pin, bool value) {
166+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
152167
I2Cdev::writeBit(devAddr, TCA6424A_RA_OUTPUT0 + (pin / 8), pin % 8, value);
153168
}
154169
/** Set all OUTPUT pins' logic levels in one bank.
155170
* @param bank Which bank to write (0/1/2 for P0*, P1*, P2* respectively)
156171
* @param value New pins' output logic level (0 or 1 for each pin)
157172
*/
158173
void TCA6424A::writeBank(uint8_t bank, uint8_t value) {
174+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
159175
I2Cdev::writeByte(devAddr, TCA6424A_RA_OUTPUT0 + bank, value);
160176
}
161177
/** Set all OUTPUT pins' logic levels in all banks.
162178
* @param banks All pins' new logic values (P00-P27) in 3-byte array
163179
*/
164180
void TCA6424A::writeAll(uint8_t *banks) {
181+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
165182
I2Cdev::writeBytes(devAddr, TCA6424A_RA_OUTPUT0 | TCA6424A_AUTO_INCREMENT, 3, banks);
166183
}
167184
/** Set all OUTPUT pins' logic levels in all banks.
@@ -170,6 +187,7 @@ void TCA6424A::writeAll(uint8_t *banks) {
170187
* @param bank2 Bank 2's new logic values (P20-P27)
171188
*/
172189
void TCA6424A::writeAll(uint8_t bank0, uint8_t bank1, uint8_t bank2) {
190+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
173191
buffer[0] = bank0;
174192
buffer[1] = bank1;
175193
buffer[2] = bank2;
@@ -182,6 +200,7 @@ void TCA6424A::writeAll(uint8_t bank0, uint8_t bank1, uint8_t bank2) {
182200
* @return Pin polarity setting (0 or 1)
183201
*/
184202
bool TCA6424A::getPinPolarity(uint16_t pin) {
203+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
185204
I2Cdev::readBit(devAddr, TCA6424A_RA_POLARITY0 + (pin / 8), pin % 8, buffer);
186205
return buffer[0];
187206
}
@@ -190,6 +209,7 @@ bool TCA6424A::getPinPolarity(uint16_t pin) {
190209
* @return 8 pins' polarity settings (0 or 1 for each pin)
191210
*/
192211
uint8_t TCA6424A::getBankPolarity(uint8_t bank) {
212+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
193213
I2Cdev::readByte(devAddr, TCA6424A_RA_POLARITY0 + bank, buffer);
194214
return buffer[0];
195215
}
@@ -198,6 +218,7 @@ uint8_t TCA6424A::getBankPolarity(uint8_t bank) {
198218
* @param banks Container for all bank's pin values (P00-P27)
199219
*/
200220
void TCA6424A::getAllPolarity(uint8_t *banks) {
221+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
201222
I2Cdev::readBytes(devAddr, TCA6424A_RA_POLARITY0, 3, banks);
202223
}
203224
/** Get all pin polarity (normal/inverted) settings from all banks.
@@ -207,6 +228,7 @@ void TCA6424A::getAllPolarity(uint8_t *banks) {
207228
* @param bank2 Container for Bank 2's pin values (P20-P27)
208229
*/
209230
void TCA6424A::getAllPolarity(uint8_t *bank0, uint8_t *bank1, uint8_t *bank2) {
231+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
210232
I2Cdev::readBytes(devAddr, TCA6424A_RA_POLARITY0, 3, buffer);
211233
*bank0 = buffer[0];
212234
*bank1 = buffer[1];
@@ -217,19 +239,22 @@ void TCA6424A::getAllPolarity(uint8_t *bank0, uint8_t *bank1, uint8_t *bank2) {
217239
* @param polarity New pin polarity setting (0 or 1)
218240
*/
219241
void TCA6424A::setPinPolarity(uint16_t pin, bool polarity) {
242+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
220243
I2Cdev::writeBit(devAddr, TCA6424A_RA_POLARITY0 + (pin / 8), pin % 8, polarity);
221244
}
222245
/** Set all pin polarity (normal/inverted) settings in one bank.
223246
* @param bank Which bank to write (0/1/2 for P0*, P1*, P2* respectively)
224247
* @return New pins' polarity settings (0 or 1 for each pin)
225248
*/
226249
void TCA6424A::setBankPolarity(uint8_t bank, uint8_t polarity) {
250+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
227251
I2Cdev::writeByte(devAddr, TCA6424A_RA_POLARITY0 + bank, polarity);
228252
}
229253
/** Set all pin polarity (normal/inverted) settings in all banks.
230254
* @param banks All pins' new logic values (P00-P27) in 3-byte array
231255
*/
232256
void TCA6424A::setAllPolarity(uint8_t *banks) {
257+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
233258
I2Cdev::writeBytes(devAddr, TCA6424A_RA_POLARITY0 | TCA6424A_AUTO_INCREMENT, 3, banks);
234259
}
235260
/** Set all pin polarity (normal/inverted) settings in all banks.
@@ -238,6 +263,7 @@ void TCA6424A::setAllPolarity(uint8_t *banks) {
238263
* @param bank2 Bank 2's new polarity values (P20-P27)
239264
*/
240265
void TCA6424A::setAllPolarity(uint8_t bank0, uint8_t bank1, uint8_t bank2) {
266+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
241267
buffer[0] = bank0;
242268
buffer[1] = bank1;
243269
buffer[2] = bank2;
@@ -250,6 +276,7 @@ void TCA6424A::setAllPolarity(uint8_t bank0, uint8_t bank1, uint8_t bank2) {
250276
* @return Pin direction setting (0 or 1)
251277
*/
252278
bool TCA6424A::getPinDirection(uint16_t pin) {
279+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
253280
I2Cdev::readBit(devAddr, TCA6424A_RA_CONFIG0 + (pin / 8), pin % 8, buffer);
254281
return buffer[0];
255282
}
@@ -258,6 +285,7 @@ bool TCA6424A::getPinDirection(uint16_t pin) {
258285
* @return 8 pins' direction settings (0 or 1 for each pin)
259286
*/
260287
uint8_t TCA6424A::getBankDirection(uint8_t bank) {
288+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
261289
I2Cdev::readByte(devAddr, TCA6424A_RA_CONFIG0 + bank, buffer);
262290
return buffer[0];
263291
}
@@ -266,6 +294,7 @@ uint8_t TCA6424A::getBankDirection(uint8_t bank) {
266294
* @param banks Container for all bank's pin values (P00-P27)
267295
*/
268296
void TCA6424A::getAllDirection(uint8_t *banks) {
297+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
269298
I2Cdev::readBytes(devAddr, TCA6424A_RA_CONFIG0, 3, banks);
270299
}
271300
/** Get all pin direction (I/O) settings from all banks.
@@ -275,6 +304,7 @@ void TCA6424A::getAllDirection(uint8_t *banks) {
275304
* @param bank2 Container for Bank 2's pin values (P20-P27)
276305
*/
277306
void TCA6424A::getAllDirection(uint8_t *bank0, uint8_t *bank1, uint8_t *bank2) {
307+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
278308
I2Cdev::readBytes(devAddr, TCA6424A_RA_CONFIG0, 3, buffer);
279309
*bank0 = buffer[0];
280310
*bank1 = buffer[1];
@@ -285,19 +315,22 @@ void TCA6424A::getAllDirection(uint8_t *bank0, uint8_t *bank1, uint8_t *bank2) {
285315
* @param direction Pin direction setting (0 or 1)
286316
*/
287317
void TCA6424A::setPinDirection(uint16_t pin, bool direction) {
318+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
288319
I2Cdev::writeBit(devAddr, TCA6424A_RA_CONFIG0 + (pin / 8), pin % 8, direction);
289320
}
290321
/** Set all pin direction (I/O) settings in one bank.
291322
* @param bank Which bank to read (0/1/2 for P0*, P1*, P2* respectively)
292323
* @param direction New pins' direction settings (0 or 1 for each pin)
293324
*/
294325
void TCA6424A::setBankDirection(uint8_t bank, uint8_t direction) {
326+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
295327
I2Cdev::writeByte(devAddr, TCA6424A_RA_CONFIG0 + bank, direction);
296328
}
297329
/** Set all pin direction (I/O) settings in all banks.
298330
* @param banks All pins' new direction values (P00-P27) in 3-byte array
299331
*/
300332
void TCA6424A::setAllDirection(uint8_t *banks) {
333+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
301334
I2Cdev::writeBytes(devAddr, TCA6424A_RA_CONFIG0 | TCA6424A_AUTO_INCREMENT, 3, banks);
302335
}
303336
/** Set all pin direction (I/O) settings in all banks.
@@ -306,6 +339,7 @@ void TCA6424A::setAllDirection(uint8_t *banks) {
306339
* @param bank2 Bank 2's new direction values (P20-P27)
307340
*/
308341
void TCA6424A::setAllDirection(uint8_t bank0, uint8_t bank1, uint8_t bank2) {
342+
mbed::ScopedLock<rtos::Mutex> lock(_wire_mtx);
309343
buffer[0] = bank0;
310344
buffer[1] = bank1;
311345
buffer[2] = bank2;

‎src/lib/ioexpander/TCA6424A.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ THE SOFTWARE.
9090

9191
class TCA6424A {
9292
public:
93-
TCA6424A();
94-
TCA6424A(uint8_t address);
93+
TCA6424A(rtos::Mutex & wire_mtx);
94+
TCA6424A(uint8_t address, rtos::Mutex & wire_mtx);
9595

9696
void initialize();
9797
bool testConnection();
@@ -133,6 +133,7 @@ class TCA6424A {
133133
void setAllDirection(uint8_t bank0, uint8_t bank1, uint8_t bank2);
134134

135135
private:
136+
rtos::Mutex & _wire_mtx;
136137
uint8_t devAddr;
137138
uint8_t buffer[3];
138139
};

0 commit comments

Comments
 (0)