P2: Support for dynamic trice and triceS macro aliases #536
P2: Support for dynamic trice and triceS macro aliases #536rokath merged 6 commits intorokath:masterfrom
Conversation
|
Thanks, Serhii, for your PR. The ./testAll.sh script passes. But I have a few questions:
An example project containing legacy code with
Keeping the til.json file clean from aliases seems to be an approach causing less trouble in the future. What, if a user calls
That is a good point. Putting the Should I wait for a PR update? |
Yes, you've understood it correctly: there's a straightforward mapping — Good catch with Hope the following example clarifies and aligns our understanding. The #define ASSERT_MESSAGE_HELPER(condition_str, file_path, line_number, ...) \
([&]() -> const char* { \
__VA_OPT__(const char* user_msg = format_message<256>(__VA_ARGS__);) \
const char* file = strrchr(file_path, '/'); \
file = file ? file + 1 : file_path; \
return format_message<384>( \
"[ASSERT] %s:%d: %s" __VA_OPT__(" -> %s") "\n", \
file, line_number, condition_str __VA_OPT__(, user_msg)); \
}())
#if !defined(TRICE_OFF) || TRICE_OFF == 0
#define ALIAS_PRINT(...) trice(__VA_ARGS__)
#define ALIAS_ASSERT(id, condition, ...) \
do { if (!(condition)) { \
const char* full_msg = ASSERT_MESSAGE_HELPER(#condition, __FILE__, __LINE__ __VA_OPT__(, __VA_ARGS__)); \
triceS(id, "%s", full_msg); \
} } while(0)
#else
#define ALIAS_PRINT(fmt, ...) Serial.print(format_message(fmt, ##__VA_ARGS__));
#define ALIAS_ASSERT(condition, ...) \
do { if (!(condition)) { \
const char* full_msg = ASSERT_MESSAGE_HELPER(#condition, __FILE__, __LINE__ __VA_OPT__(, __VA_ARGS__)); \
Serial.print(full_msg); \
} } while(0)
#endif
Yes, if I’m not missing anything, that basically requires either a more sophisticated mapping method (like a config file), which increases the learning curve, or—as I briefly mentioned earlier—more advanced tooling that can potentially track macro expansion and perform “magic,” but at the cost of added complexity in the toolchain execution:
That's a bit odd — the untrimmed execution checks don't seem to fail anymore, but there are still other errors. Sorry for the confusion. I've attached the log in case it helps identify the issues. I'm simply running ./testAll.sh as-is testAll.log
At this point, it might be simpler to just remove it. Do you want me to do it? |
Yes, please. There is one more reason:
./../testdata/triceCheck.c:2761:8: error: incompatible pointer to integer conversion passing 'char[70]' to parameter of type 'uint16_t' (aka 'unsigned short') [-Wint-conversion]
2761 | Trice("att:MyStructEvaluationFunction(json:ExA{Apple:%d, Birn:%u, Fisch:%f}\n", Ex.Apple, Ex.Birn, aFloat(Ex.Fish));This tells us, that in triceCheck.c line 2761 is no
I added for now a few If the problem still persists, please try the released Trice tool version 1.0 for the ID handling. Maybe the parsing now is somehow different. Concerning your code snippets: I understand the need to merge legacy code without or maybe only little changing it. In #527 I mentioned:
This could be a parallel way to go, but the |
|
@rokath Thank you for adding the As requested, I removed Alias serialization from til.json and committed that change separately (436fb05). I chose not to clone G0B1_inst here (we can do that before merging if needed) to keep the review focused on my changes. Instead, I updated G0B1_inst with custom macro examples. These are similar but differ slightly since sprintf isn’t available; nanoprintf was added to address this. I can't confirm full functionality without hardware, but it should illustrate the need for "%s" enforcement—happy to get suggestions (966b63f). It seems It also fails to build The When you have a moment, please review the changes and let me know if you have any questions, suggestions, or if I’ve overlooked anything. Disclaimer: I'm not an embedded specialist—my experience is primarily with CPUs, not MCUs—so I lack the necessary hardware to verify and could be off on embedded-specific details. Please forgive any errors. |
|
Hi Serhii, I just looked at your logs. build.log line 15: This is still the mentioned issue. So it seems to be no timing issue because of the inserted sleeps. Questions:
When I look at testAll.log, I see: ID 13003 inside examples/G0B1_inst/Core/Src/main.c line 127 refers to {triceS heFastFoundAnswer == theRightAnswe CUSTOM_ASSERT} but is used inside til.json for {triceS %s CUSTOM_ASSERT} - setting it to 0.
ID 13004 inside examples/G0B1_inst/Core/Src/main.c line 130 refers to {triceS heFastFoundAnswer == theRightAnswer, (char*)theQuestion CUSTOM_ASSERT} but is used inside til.json for {triceS %s CUSTOM_ASSERT} - setting it to 0.
ID 13005 inside examples/G0B1_inst/Core/Src/main.c line 133 refers to {triceS heFastFoundAnswer == theRightAnswer, (char*)"'%s' Am, it is %d CUSTOM_ASSERT} but is used inside til.json for {triceS %s CUSTOM_ASSERT} - setting it to 0.
ID 13006 inside examples/G0B1_inst/Core/Src/main.c line 336 refers to {TRice msg:StartDefaultTask\n } but is used inside til.json for {TRice msg:StartDefaultTask\n CUSTOM_ASSERT} - setting it to 0.
ID 13007 inside examples/G0B1_inst/Core/Src/main.c line 363 refers to {TRice msg:StartTask02:Diagnostics and TriceTransfer\n } but is used inside til.json for {TRice msg:StartTask02:Diagnostics and TriceTransfer\n CUSTOM_ASSERT} - setting it to 0.This noise tells us, that you have the problems with by the PR modified files. For the errors starting in line 875 you got a hint in line 900: Recommendation:
The testAll.sh script now displays some context information. Probably on Wednesday I will be able to have a closer look to your code. |
|
I am sorry Serhii, today I was only working on TriceUserManual.md/#trice-structured-logging which was triggered by #531 and could not proceed with your PR. Please be patient, I will do my best asap. Even it is off-topic here I would be happy to get your critical feedback on the mentioned draft. |
Info about error in file triceCheck.c line 2761Accidentally there is a missing closing bracket inside of the format string at this position. The legacy parsing tolerated it, because the user is allowed to write whatever he/she wants inside the Trice format string. It is the users world. The PR536 parsing behaves differently and that is the reason, the test fails here. File Info about build errorsThere is now a file Over the weekend I hopefully will be able to look deeper in the proposed changes. |
|
Thomas, thank you.. I will look at all of it over the weekend too (including the draft). |
|
Hi Serhii, Trying to run Core/Src/main.c: In function 'StartDefaultTask':
Core/Src/main.c:337:33: error: macro "TRice" requires 3 arguments, but only 1 given
337 | TRice("msg:StartDefaultTask\n");In line 337 no iD gets inserted anymore. If I build the Trice tool in the main branch and run To proceed, please, I need your help. I guess you have to check the PR536 parser functionality. As of my knowledge right now, after adapting Editing the main branch in a way to just change the parser functionality according PR536 somehow updated, should firstly give success on Then adding the G0B1_inst changes would be the next step. Looking forward to your response. |
…riceAlias, and expanded tests for alias handling.
…escaped quotes and parentheses in string literals.
|
Thomas, I have pushed some fixes, please see below, so far it seems that G0B1_inst example is the only remaining:
Thanks for flagging the issue in triceCheck.c:2761. I've addressed it with a separate, granular commit. If you'd prefer to apply the fix directly to master/upstream, I can roll back my change and rebase accordingly.
I've updated build_environment.sh to detect the gcc-arm-embedded version installed via Homebrew. While this isn't a universal solution, it fits with the existing use of Brew in the project.
Fixed. The root cause was improper handling of parentheses within string literals. I have added two more test cases to match test. Please see go test ./...
go: downloading go.bug.st/serial v1.6.0
go: downloading github.com/rokath/cobs v0.0.0-20230425030040-4ebbe9b903b9
go: downloading github.com/spf13/afero v1.9.5
go: downloading github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
go: downloading github.com/rokath/tcobs v0.9.1
go: downloading github.com/tj/assert v0.0.3
go: downloading github.com/kr/pretty v0.1.0
go: downloading golang.org/x/crypto v0.35.0
go: downloading github.com/stretchr/testify v1.8.4
go: downloading github.com/udhos/equalfile v0.3.0
go: downloading github.com/fsnotify/fsnotify v1.6.0
go: downloading github.com/pkg/errors v0.9.1
go: downloading github.com/mattn/go-colorable v0.1.13
go: downloading golang.org/x/text v0.22.0
go: downloading github.com/kr/text v0.1.0
go: downloading github.com/pmezard/go-difflib v1.0.0
go: downloading github.com/davecgh/go-spew v1.1.1
go: downloading gopkg.in/yaml.v3 v3.0.1
go: downloading golang.org/x/sys v0.30.0
go: downloading github.com/creack/goselect v0.1.2
go: downloading github.com/mattn/go-isatty v0.0.19
ok github.com/rokath/trice/cmd/trice 1.203s
ok github.com/rokath/trice/internal/args 0.838s
ok github.com/rokath/trice/internal/charDecoder 0.579s
ok github.com/rokath/trice/internal/com 3.860s
ok github.com/rokath/trice/internal/decoder 0.706s
? github.com/rokath/trice/internal/do [no test files]
ok github.com/rokath/trice/internal/dumpDecoder 0.822s
ok github.com/rokath/trice/internal/emitter 0.948s
ok github.com/rokath/trice/internal/id 3.676s
ok github.com/rokath/trice/internal/keybcmd 1.178s
ok github.com/rokath/trice/internal/link 1.047s
ok github.com/rokath/trice/internal/receiver 0.901s
? github.com/rokath/trice/internal/translator [no test files]
ok github.com/rokath/trice/internal/trexDecoder 0.853s
? github.com/rokath/trice/pkg/ant [no test files]
ok github.com/rokath/trice/pkg/cipher 0.728s
ok github.com/rokath/trice/pkg/endian 1.077s
ok github.com/rokath/trice/pkg/msg 0.968s
ok github.com/rokath/trice/pkg/tst 1.196sHere is a recent testAll.log |
|
SORRYThe observations, I told you where accidently done not with a fresh build Trice tool! Tomorrow you will get an update! |
|
Things are much better now :-) I am still investigating but a small problem is this test, I added into ./repos/wt_trice_pr536b/internal/id/insertIDs_test.go locally: func TestInsertKnownID2(t *testing.T) {
defer Setup(t)() // This executes Setup(t) and puts the returned function into the defer list.
// create existing li.json file
exsLI := `{
"55": {
"File": "file1.c",
"Line": 3
},
"77": {
"File": "file1.c",
"Line": 2
},
"999": {
"File": "fileX.c",
"Line": 2
}
}`
assert.Nil(t, FSys.WriteFile(LIFnJSON, []byte(exsLI), 0777))
// create existing til.json file
exsTIL := `{
"55": {
"Type": "trice",
"Strg": "msg:value=%d\\n"
},
"77": {
"Type": "trice",
"Strg": "%x"
}
}`
assert.Nil(t, FSys.WriteFile(FnJSON, []byte(exsTIL), 0777))
// create src file1
src1 := `
trice("%x", 123 );
trice("msg:value=%d\n", 123 );
`
assert.Nil(t, FSys.WriteFile("file1.c", []byte(src1), 0777))
// action
assert.Nil(t, args.Handler(W, FSys, []string{"trice", "insert", "-alias", "log", "-src", "file1.c", "-IDMin", "100", "-IDMax", "999", "-IDMethod", "downward", "-til", FnJSON, "-li", LIFnJSON}))
// check modified src file1
expSrc1 := `
trice(iD(77), "%x", 123 );
trice(iD(55), "msg:value=%d\n", 123 );
`
actSrc1, e := FSys.ReadFile("file1.c")
assert.Nil(t, e)
assert.Equal(t, expSrc1, string(actSrc1))
// check til.json
actTIL, e := FSys.ReadFile(FnJSON)
assert.Nil(t, e)
assert.Equal(t, exsTIL, string(actTIL))
// check li.json
actLI, e := FSys.ReadFile(LIFnJSON)
assert.Nil(t, e)
assert.Equal(t, exsLI, string(actLI))
// cleanup
FSys.Remove(FnJSON)
FSys.Remove(LIFnJSON)
FSys.RemoveAll(UserHomeDir)
}In this form the test passes. I would expect a pass as well, when in the src file one "trice" gets replaced by "log". But in that case a new ID is assigned, what I consider as a bug. |
|
Should I merge for now, or should we wait? Some details need a better documentation. I could try to write s.th. but probably you can do this better. |
|
Sure, I can take care of it—just let me know which parts you'd like documented. |
Ok, then I will merge tomorrow and start to document and also try to debug the small problem. |
|
Hi Serhii, please one question: Why did you add Best, Thomas ms@Mac trice % ls /Library/Developer/CommandLineTools/usr/bin
2to3 clangd git-upload-pack llvm-size Rez swift-package-collection
2to3-3.9 cmpdylib gm4 lorder rpcgen swift-package-registry
aarch64-swift-linux-musl-clang.cfg codesign_allocate gnumake m4 scalar swift-plugin-server
aarch64-swift-linux-musl-clang++.cfg codesign_allocate-p gperf make segedit swift-run
ar cpp hdxml2manxml mig SetFile swift-sdk
as crashlog headerdoc2html nm size swift-stdlib-tool
asa ctags indent nm-classic size-classic swift-symbolgraph-extract
bison ctf_insert install_name_tool nmedit sourcekit-lsp swift-synthesize-interface
bitcode_strip DeRez ld notarytool SplitForks swift-test
bm4 dsymutil ld-classic objdump stapler swiftc
byacc dwarfdump lex otool strings tapi
c++ dyld_info libtool otool-classic strip tapi-analyze
c++filt flex lipo pagestuff swift unifdef
c89 flex++ lldb pip3 swift-api-digester unifdefall
c99 g++ lldb-dap pip3.9 swift-build unwinddump
cache-build-session gatherheaderdoc llvm-cov pydoc3 swift-cache-tool vtool
cc gcc llvm-cxxfilt pydoc3.9 swift-demangle x86_64-swift-linux-musl-clang.cfg
clang gcov llvm-dwarfdump python3 swift-driver x86_64-swift-linux-musl-clang++.cfg
clang-cache GetFileInfo llvm-nm python3.9 swift-experimental-sdk xcindex-test
clang-format git llvm-objdump ranlib swift-format xml2man
clang-format-diff.py git-receive-pack llvm-otool readtapi swift-frontend yacc
clang-stat-cache git-shell llvm-profdata ResMerger swift-help
clang++ git-upload-archive llvm-readtapi resolveLinks swift-package |
|
Thomas, thank you for reviewing and merging the PR. I'm not quite sure about
|
|
For some reason I added this line and forgot about it. Probably I was extending the path for objcopy. It is commented out now and we will see.... Sorry for the noise. |


What This PR Adds
This is a follow-up to #533. It enforces the consistent use of the "%s" format in all triceS aliases and fixes that behavior in the newly added test case.
The following simplified example reflects a real use case where custom macros wrap formatting logic:
The problem statement
Determining the Strg argument reliably is challenging due to:
For instance, custom macros like these show the variability:
Improving this would likely require Clang integration—adding complexity (e.g., full build flags, complete source context)—whereas Trice’s current regex-based approach remains lightweight and simple to use.
Implementation details:
matchTrice()was re-implemented to improve robustness. It now:This approach simplifies the logic and allows the parser to skip invalid or partial matches without aborting, enabling continued scanning of the file for valid constructs.