Incorrect log publisher topic by commander modules

[tecnic08]

Describe the bug
I have a problem where my global position is invalid and failsafe is triggered. I checked the source code and the failsafe should be triggered by commander module.
However, logged message indicated that it was published by commander_tests. Which wasn't running at that time.

I've check other people log and it seems that they all have the same problem in both SITL and actual hardware such as
In SITL: #9178
In hardware: https://logs.px4.io/plot_app?log=e226431c-9515-456c-8d45-d2fd22d0d68b
In my case, hardware (See Logged Messages at the time of 00:11:04
[commander_tests] Failsafe enabled: no global position): https://logs.px4.io/plot_app?log=1e614d6c-d948-43c2-ba40-b0312aceaa83

All log indicated [commander_tests] Failsafe enabled: no global position (or local) which should be [commander] instead.

After some digging and searching through the code. I found that state_machine_helper.cpp and PreflightCheck.cpp is being included in SRCS of CMakeLists.txt of both commander_test and commander. Moreover, commander_test is being compiled and linked before commander. Which might be the possible cause of this bug.

I am using a derived v1.8.2, but this part is untouched. Also, v1.9.2 still exhibit the same code.

commander_test cmake: https://github.com/PX4/Firmware/blob/v1.9.2/src/modules/commander/commander_tests/CMakeLists.txt
commander cmake: https://github.com/PX4/Firmware/blob/v1.9.2/src/modules/commander/CMakeLists.txt

I think mavlink log pub handler might incorrectly assume the source of the message based on the first module name that it is being compiled.

To Reproduce
Make anything that triggers a logged message that output from state_machine_helper.cpp or PreflightCheck.cpp inside src/modules/commander. It will be logged as [commander_tests]

Expected behavior
Log should indicate that publisher is [commander]

Additional context
I know that my vibration is severe, but that's not the case for this bug.

[julianoes]

Thanks for the issue, that's interesting!

[julianoes]

The module name is set in this include:
https://github.com/PX4/Firmware/blob/166639be3a21be00ef80eb619aa23fbc8751ac3e/src/platforms/px4_log.h#L164

I'm not sure how this can happen, given that generally it is correct.

As a fix we could move the common files into a library and then link to that one in the module and in tests. It's not very pretty though because then the module name is e.g. commander_lib.

[tecnic08]

As a fix we could move the common files into a library and then link to that one in the module and in tests. It's not very pretty though because then the module name is e.g. commander_lib.

That should do the job.

[tecnic08]

@julianoes Will there be a fix to this issue?

[julianoes]

Can't promise a time sorry. Feel free to submit a pull request.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.