fix thread handling of node_status_publisher.

[s-azumi]

Status

PRODUCTION / DEVELOPMENT

Description

This tries to solve autowarefoundation/autoware_ai#589.

#2028 couldn't path the tests because of autoware_health_checker::NodeStatusPublisher.
I think its cause is that the thread is not dead during the test.

[sgermanserrano]

@s-azumi @gbiggs @kfunaoka @hakuturu583
Thread handling in the autoware_health_checker node is causing issues in the packages it affects. As @dejanpan comments in #1943 there are no comments or design document for the new feature.

[sgermanserrano]

If my understanding is correct the fix proposed in this PR will work for just 1 node making use of the health checker as the publish_thread_ variable will be overwritten if 2 nodes make use of the ENABLE method

[sgermanserrano]

If the autoware_health_checker package is performing the same tasks as diagnostic_aggregator I'd suggest using that one instead to reduce maintainability needs

[hakuturu583]

@sgermanserrano I posted design issun for this feature. But no one gave opinion to us.

[hakuturu583]

＠sgermanserrano diagnostic_aggrigator only handles hardware errors.
It does not handle software diagnostic function.

[kfunaoka]

@hakuturu583 It seems each node has an instance of NodeStatusPublisher. In other words, each node launches only one thread in NodeStatusPublisher::ENABLE(). Is my understanding correct?

[sgermanserrano]

＠sgermanserrano diagnostic_aggrigator only handles hardware errors.
It does not handle software diagnostic function.

@hakuturu583 thanks for the clarification. Is it then close to what bond does?

I'm reading autowarefoundation/autoware_ai#494 but I cannot find how the new feature works, can you provide a ReadMe within the package? Also, how is thread completion handled? Would this PR #2058 address that? are there any other conditions (apart from the Destructor) when .join should be called?

[kfunaoka]

Since this bug is hotfix level, I've looked into the code. At 6a5c720, NodeStatusPublisher is instantiated only once in each node, and NodeStatusPublisher::ENABLE() is called only once in each node. This pull request also fixes the CI error in #2028.

@hakuturu583 Would you add README.md in new designated pull request?

[kfunaoka]

I've requested README.md and answers at https://github.com/CPFL/Autoware/issues/1860#issuecomment-469940811.