Skip to content

Commit 15b3135

Browse files
sjasonsmithvgadreau
sjasonsmith
authored and
vgadreau
committed
Catch a TMC address conflict early (MarlinFirmware#19458)
1 parent f344b69 commit 15b3135

File tree

4 files changed

+117
-3
lines changed

4 files changed

+117
-3
lines changed

‎Marlin/src/module/stepper/trinamic.cpp

+105
Original file line numberDiff line numberDiff line change
@@ -209,113 +209,145 @@ enum StealthIndex : uint8_t { STEALTH_AXIS_XY, STEALTH_AXIS_Z, STEALTH_AXIS_E };
209209
#if AXIS_HAS_UART(X)
210210
#ifdef X_HARDWARE_SERIAL
211211
TMC_UART_DEFINE(HW, X, X);
212+
#define X_HAS_HW_SERIAL 1
212213
#else
213214
TMC_UART_DEFINE(SW, X, X);
215+
#define X_HAS_SW_SERIAL 1
214216
#endif
215217
#endif
216218
#if AXIS_HAS_UART(X2)
217219
#ifdef X2_HARDWARE_SERIAL
218220
TMC_UART_DEFINE(HW, X2, X);
221+
#define X2_HAS_HW_SERIAL 1
219222
#else
220223
TMC_UART_DEFINE(SW, X2, X);
224+
#define X2_HAS_SW_SERIAL 1
221225
#endif
222226
#endif
223227
#if AXIS_HAS_UART(Y)
224228
#ifdef Y_HARDWARE_SERIAL
225229
TMC_UART_DEFINE(HW, Y, Y);
230+
#define Y_HAS_HW_SERIAL 1
226231
#else
227232
TMC_UART_DEFINE(SW, Y, Y);
233+
#define Y_HAS_SW_SERIAL 1
228234
#endif
229235
#endif
230236
#if AXIS_HAS_UART(Y2)
231237
#ifdef Y2_HARDWARE_SERIAL
232238
TMC_UART_DEFINE(HW, Y2, Y);
239+
#define Y2_HAS_HW_SERIAL 1
233240
#else
234241
TMC_UART_DEFINE(SW, Y2, Y);
242+
#define Y2_HAS_SW_SERIAL 1
235243
#endif
236244
#endif
237245
#if AXIS_HAS_UART(Z)
238246
#ifdef Z_HARDWARE_SERIAL
239247
TMC_UART_DEFINE(HW, Z, Z);
248+
#define Z_HAS_HW_SERIAL 1
240249
#else
241250
TMC_UART_DEFINE(SW, Z, Z);
251+
#define Z_HAS_SW_SERIAL 1
242252
#endif
243253
#endif
244254
#if AXIS_HAS_UART(Z2)
245255
#ifdef Z2_HARDWARE_SERIAL
246256
TMC_UART_DEFINE(HW, Z2, Z);
257+
#define Z2_HAS_HW_SERIAL 1
247258
#else
248259
TMC_UART_DEFINE(SW, Z2, Z);
260+
#define Z2_HAS_SW_SERIAL 1
249261
#endif
250262
#endif
251263
#if AXIS_HAS_UART(Z3)
252264
#ifdef Z3_HARDWARE_SERIAL
253265
TMC_UART_DEFINE(HW, Z3, Z);
266+
#define Z3_HAS_HW_SERIAL 1
254267
#else
255268
TMC_UART_DEFINE(SW, Z3, Z);
269+
#define Z3_HAS_SW_SERIAL 1
256270
#endif
257271
#endif
258272
#if AXIS_HAS_UART(Z4)
259273
#ifdef Z4_HARDWARE_SERIAL
260274
TMC_UART_DEFINE(HW, Z4, Z);
275+
#define Z4_HAS_HW_SERIAL 1
261276
#else
262277
TMC_UART_DEFINE(SW, Z4, Z);
278+
#define Z4_HAS_SW_SERIAL 1
263279
#endif
264280
#endif
265281
#if AXIS_HAS_UART(E0)
266282
#ifdef E0_HARDWARE_SERIAL
267283
TMC_UART_DEFINE_E(HW, 0);
284+
#define E0_HAS_HW_SERIAL 1
268285
#else
269286
TMC_UART_DEFINE_E(SW, 0);
287+
#define E0_HAS_SW_SERIAL 1
270288
#endif
271289
#endif
272290
#if AXIS_HAS_UART(E1)
273291
#ifdef E1_HARDWARE_SERIAL
274292
TMC_UART_DEFINE_E(HW, 1);
293+
#define E1_HAS_HW_SERIAL 1
275294
#else
276295
TMC_UART_DEFINE_E(SW, 1);
296+
#define E1_HAS_SW_SERIAL 1
277297
#endif
278298
#endif
279299
#if AXIS_HAS_UART(E2)
280300
#ifdef E2_HARDWARE_SERIAL
281301
TMC_UART_DEFINE_E(HW, 2);
302+
#define E2_HAS_HW_SERIAL 1
282303
#else
283304
TMC_UART_DEFINE_E(SW, 2);
305+
#define E2_HAS_SW_SERIAL 1
284306
#endif
285307
#endif
286308
#if AXIS_HAS_UART(E3)
287309
#ifdef E3_HARDWARE_SERIAL
288310
TMC_UART_DEFINE_E(HW, 3);
311+
#define E3_HAS_HW_SERIAL 1
289312
#else
290313
TMC_UART_DEFINE_E(SW, 3);
314+
#define E3_HAS_SW_SERIAL 1
291315
#endif
292316
#endif
293317
#if AXIS_HAS_UART(E4)
294318
#ifdef E4_HARDWARE_SERIAL
295319
TMC_UART_DEFINE_E(HW, 4);
320+
#define E4_HAS_HW_SERIAL 1
296321
#else
297322
TMC_UART_DEFINE_E(SW, 4);
323+
#define E4_HAS_SW_SERIAL 1
298324
#endif
299325
#endif
300326
#if AXIS_HAS_UART(E5)
301327
#ifdef E5_HARDWARE_SERIAL
302328
TMC_UART_DEFINE_E(HW, 5);
329+
#define E5_HAS_HW_SERIAL 1
303330
#else
304331
TMC_UART_DEFINE_E(SW, 5);
332+
#define E5_HAS_SW_SERIAL 1
305333
#endif
306334
#endif
307335
#if AXIS_HAS_UART(E6)
308336
#ifdef E6_HARDWARE_SERIAL
309337
TMC_UART_DEFINE_E(HW, 6);
338+
#define E6_HAS_HW_SERIAL 1
310339
#else
311340
TMC_UART_DEFINE_E(SW, 6);
341+
#define E6_HAS_SW_SERIAL 1
312342
#endif
313343
#endif
314344
#if AXIS_HAS_UART(E7)
315345
#ifdef E7_HARDWARE_SERIAL
316346
TMC_UART_DEFINE_E(HW, 7);
347+
#define E7_HAS_HW_SERIAL 1
317348
#else
318349
TMC_UART_DEFINE_E(SW, 7);
350+
#define E7_HAS_SW_SERIAL 1
319351
#endif
320352
#endif
321353

@@ -769,4 +801,77 @@ void reset_trinamic_drivers() {
769801
stepper.set_directions();
770802
}
771803

804+
// TMC Slave Address Conflict Detection
805+
//
806+
// Conflict detection is performed in the following way. Similar methods are used for
807+
// hardware and software serial, but the implementations are indepenent.
808+
//
809+
// 1. Populate a data structure with UART parameters and addresses for all possible axis.
810+
// If an axis is not in use, populate it with recognizable placeholder data.
811+
// 2. For each axis in use, static_assert using a constexpr function, which counts the
812+
// number of matching/conflicting axis. If the value is not exactly 1, fail.
813+
814+
#if ANY_AXIS_HAS(HW_SERIAL)
815+
// Hardware serial names are compared as strings, since actually resolving them cannot occur in a constexpr.
816+
// Using a fixed-length character array for the port name allows this to be constexpr compatible.
817+
struct SanityHwSerialDetails { const char port[20]; uint32_t address; };
818+
#define TMC_HW_DETAIL_ARGS(A) TERN(A##_HAS_HW_SERIAL, STRINGIFY(A##_HARDWARE_SERIAL), ""), TERN0(A##_HAS_HW_SERIAL, A##_SLAVE_ADDRESS)
819+
#define TMC_HW_DETAIL(A) {TMC_HW_DETAIL_ARGS(A)}
820+
constexpr SanityHwSerialDetails sanity_tmc_hw_details[] = {
821+
TMC_HW_DETAIL(X), TMC_HW_DETAIL(X2),
822+
TMC_HW_DETAIL(Y), TMC_HW_DETAIL(Y2),
823+
TMC_HW_DETAIL(Z), TMC_HW_DETAIL(Z2), TMC_HW_DETAIL(Z3), TMC_HW_DETAIL(Z4),
824+
TMC_HW_DETAIL(E0), TMC_HW_DETAIL(E1), TMC_HW_DETAIL(E2), TMC_HW_DETAIL(E3), TMC_HW_DETAIL(E4), TMC_HW_DETAIL(E5), TMC_HW_DETAIL(E6), TMC_HW_DETAIL(E7)
825+
};
826+
827+
// constexpr compatible string comparison
828+
constexpr bool str_eq_ce(const char * a, const char * b) {
829+
return *a == *b && (*a == '\0' || str_eq_ce(a+1,b+1));
830+
}
831+
832+
constexpr bool sc_hw_done(size_t start, size_t end) { return start == end; }
833+
constexpr bool sc_hw_skip(const char* port_name) { return !(*port_name); }
834+
constexpr bool sc_hw_match(const char* port_name, uint32_t address, size_t start, size_t end) {
835+
return !sc_hw_done(start, end) && !sc_hw_skip(port_name) && (address == sanity_tmc_hw_details[start].address && str_eq_ce(port_name, sanity_tmc_hw_details[start].port));
836+
}
837+
constexpr int count_tmc_hw_serial_matches(const char* port_name, uint32_t address, size_t start, size_t end) {
838+
return sc_hw_done(start, end) ? 0 : ((sc_hw_skip(port_name) ? 0 : (sc_hw_match(port_name, address, start, end) ? 1 : 0)) + count_tmc_hw_serial_matches(port_name, address, start + 1, end));
839+
}
840+
841+
#define TMC_HWSERIAL_CONFLICT_MSG(A) STRINGIFY(A) "_SLAVE_ADDRESS conflicts with another driver using the same " STRINGIFY(A) "_HARDWARE_SERIAL"
842+
#define SA_NO_TMC_HW_C(A) static_assert(1 >= count_tmc_hw_serial_matches(TMC_HW_DETAIL_ARGS(A), 0, COUNT(sanity_tmc_hw_details)), TMC_HWSERIAL_CONFLICT_MSG(A));
843+
SA_NO_TMC_HW_C(X);SA_NO_TMC_HW_C(X2);
844+
SA_NO_TMC_HW_C(Y);SA_NO_TMC_HW_C(Y2);
845+
SA_NO_TMC_HW_C(Z);SA_NO_TMC_HW_C(Z2);SA_NO_TMC_HW_C(Z3);SA_NO_TMC_HW_C(Z4);
846+
SA_NO_TMC_HW_C(E0);SA_NO_TMC_HW_C(E1);SA_NO_TMC_HW_C(E2);SA_NO_TMC_HW_C(E3);SA_NO_TMC_HW_C(E4);SA_NO_TMC_HW_C(E5);SA_NO_TMC_HW_C(E6);SA_NO_TMC_HW_C(E7);
847+
#endif
848+
849+
#if ANY_AXIS_HAS(SW_SERIAL)
850+
struct SanitySwSerialDetails { int32_t txpin; int32_t rxpin; uint32_t address; };
851+
#define TMC_SW_DETAIL_ARGS(A) TERN(A##_HAS_SW_SERIAL, A##_SERIAL_TX_PIN, -1), TERN(A##_HAS_SW_SERIAL, A##_SERIAL_RX_PIN, -1), TERN0(A##_HAS_SW_SERIAL, A##_SLAVE_ADDRESS)
852+
#define TMC_SW_DETAIL(A) TMC_SW_DETAIL_ARGS(A)
853+
constexpr SanitySwSerialDetails sanity_tmc_sw_details[] = {
854+
TMC_SW_DETAIL(X), TMC_SW_DETAIL(X2),
855+
TMC_SW_DETAIL(Y), TMC_SW_DETAIL(Y2),
856+
TMC_SW_DETAIL(Z), TMC_SW_DETAIL(Z2), TMC_SW_DETAIL(Z3), TMC_SW_DETAIL(Z4),
857+
TMC_SW_DETAIL(E0), TMC_SW_DETAIL(E1), TMC_SW_DETAIL(E2), TMC_SW_DETAIL(E3), TMC_SW_DETAIL(E4), TMC_SW_DETAIL(E5), TMC_SW_DETAIL(E6), TMC_SW_DETAIL(E7)
858+
};
859+
860+
constexpr bool sc_sw_done(size_t start, size_t end) { return start == end; }
861+
constexpr bool sc_sw_skip(int32_t txpin) { return txpin < 0; }
862+
constexpr bool sc_sw_match(int32_t txpin, int32_t rxpin, uint32_t address, size_t start, size_t end) {
863+
return !sc_sw_done(start, end) && !sc_sw_skip(txpin) && (txpin == sanity_tmc_sw_details[start].txpin || rxpin == sanity_tmc_sw_details[start].rxpin) && (address == sanity_tmc_sw_details[start].address);
864+
}
865+
constexpr int count_tmc_sw_serial_matches(int32_t txpin, int32_t rxpin, uint32_t address, size_t start, size_t end) {
866+
return sc_sw_done(start, end) ? 0 : ((sc_sw_skip(txpin) ? 0 : (sc_sw_match(txpin, rxpin, address, start, end) ? 1 : 0)) + count_tmc_sw_serial_matches(txpin, rxpin, address, start + 1, end));
867+
}
868+
869+
#define TMC_SWSERIAL_CONFLICT_MSG(A) STRINGIFY(A) "_SLAVE_ADDRESS conflicts with another driver using the same " STRINGIFY(A) "_SERIAL_RX_PIN or " STRINGIFY(A) "_SERIAL_TX_PIN"
870+
#define SA_NO_TMC_SW_C(A) static_assert(1 >= count_tmc_sw_serial_matches(TMC_SW_DETAIL_ARGS(A), 0, COUNT(sanity_tmc_sw_details)), TMC_SWSERIAL_CONFLICT_MSG(A));
871+
SA_NO_TMC_SW_C(X);SA_NO_TMC_SW_C(X2);
872+
SA_NO_TMC_SW_C(Y);SA_NO_TMC_SW_C(Y2);
873+
SA_NO_TMC_SW_C(Z);SA_NO_TMC_SW_C(Z2);SA_NO_TMC_SW_C(Z3);SA_NO_TMC_SW_C(Z4);
874+
SA_NO_TMC_SW_C(E0);SA_NO_TMC_SW_C(E1);SA_NO_TMC_SW_C(E2);SA_NO_TMC_SW_C(E3);SA_NO_TMC_SW_C(E4);SA_NO_TMC_SW_C(E5);SA_NO_TMC_SW_C(E6);SA_NO_TMC_SW_C(E7);
875+
#endif
876+
772877
#endif // HAS_TRINAMIC_CONFIG

‎buildroot/bin/opt_set

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ set -e
66
SED=$(which gsed || which sed)
77

88
# Logic for returning nonzero based on answer here: https://stackoverflow.com/a/15966279/104648
9-
eval "${SED} -i '/\(\/\/\)*\([[:blank:]]*\)\(#define \b${1}\b\).*$/{s//\2\3 ${2}/;h};\${x;/./{x;q0};x;q9}' Marlin/Configuration.h" ||
10-
eval "${SED} -i '/\(\/\/\)*\([[:blank:]]*\)\(#define \b${1}\b\).*$/{s//\2\3 ${2}/;h};\${x;/./{x;q0};x;q9}' Marlin/Configuration_adv.h" ||
9+
eval "${SED} -i '/\(\/\/\)*\([[:blank:]]*\)\(#define\s\+\b${1}\b\).*$/{s//\2\3 ${2}/;h};\${x;/./{x;q0};x;q9}' Marlin/Configuration.h" ||
10+
eval "${SED} -i '/\(\/\/\)*\([[:blank:]]*\)\(#define\s\+\b${1}\b\).*$/{s//\2\3 ${2}/;h};\${x;/./{x;q0};x;q9}' Marlin/Configuration_adv.h" ||
1111
eval "echo '#define ${@}' >>Marlin/Configuration_adv.h" ||
1212
(echo "ERROR: opt_set Can't set or add ${1}" >&2 && exit 9)

‎buildroot/tests/STM32F103RC_btt-tests

+6-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,12 @@ opt_set SERIAL_PORT_2 -1
1616
opt_set X_DRIVER_TYPE TMC2209
1717
opt_set Y_DRIVER_TYPE TMC2209
1818
opt_set Z_DRIVER_TYPE TMC2209
19-
opt_set E_DRIVER_TYPE TMC2209
19+
opt_set E0_DRIVER_TYPE TMC2209
20+
opt_set X_SLAVE_ADDRESS 0
21+
opt_set Y_SLAVE_ADDRESS 1
22+
opt_set Z_SLAVE_ADDRESS 2
23+
opt_set E0_SLAVE_ADDRESS 3
24+
2025
exec_test $1 $2 "BigTreeTech SKR Mini E3 1.0 - Basic Config with TMC2209 HW Serial"
2126

2227
# clean up

‎buildroot/tests/esp32-tests

+4
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ opt_set X_HARDWARE_SERIAL Serial1
3030
opt_set Y_HARDWARE_SERIAL Serial1
3131
opt_set Z_HARDWARE_SERIAL Serial1
3232
opt_set E0_HARDWARE_SERIAL Serial1
33+
opt_set X_SLAVE_ADDRESS 0
34+
opt_set Y_SLAVE_ADDRESS 1
35+
opt_set Z_SLAVE_ADDRESS 2
36+
opt_set E0_SLAVE_ADDRESS 3
3337
opt_enable HOTEND_IDLE_TIMEOUT
3438
exec_test $1 $2 "ESP32, TMC HW Serial, Hotend Idle"
3539

0 commit comments

Comments
 (0)