Implement rclcpp::is_initialized()

[chapulina]

Addresses #518.

• Named the function is_initialized instead of isInitialized because that seemed to be the style throughout the utilities
• It turns out that rcl_ok was already doing what was desired, so rclcpp::is_initialized is just a wrapper around that.
• Added new test file so it could run isolated

[mikaelarguedas]

hmm that's where the naming will get confusing..
Change is good as rcl_ok actually means "is initialized". It's just confusing that it doesn't have the same semantic as rclcpp::ok.

👍 lgtm with green CI

[wjwwood]

@mikaelarguedas that's why originally I had suggested that should be the semantic of rclcpp::ok(): #3 (comment)

I'm fine with having an is_initialized() as @chapulina has implemented here, but I think ok should probably also change in rclcpp to match how it worked (or at least how it is documented) in roscpp.

[wjwwood]

Or the name in rcl should also change to rcl_is_initialized(). That would also be ok by me. At the very least we should ticket the need to change the name of rcl_ok if we merge this without doing it directly.

[wjwwood]

I would expect is_initialized() to be false after shutdown, is that not the expectation? I believe rcl_ok should return false if you call rcl_shutdown (maybe we're not doing that properly yet).

---

Also, what happens if you do:

• init
• shutdown
• init
• EXPECT_TRUE(rclcpp::is_initialized())

I'd then expect it to be true, but not before doing the second init.

[wjwwood]

Sorry the previous comment was really two different questions, let me edit it real quick.

[wjwwood]

Yeah, it looks like we're not calling rcl_shutdown ever, which until now hasn't had any consequence (none of our middlewares do anything on rcl_shutdown/rmw_shutdown):

rclcpp/rclcpp/src/rclcpp/utilities.cpp

Lines 270 to 288 in 8f2052d

	void
	rclcpp::shutdown()
	{
	trigger_interrupt_guard_condition(SIGINT);
	{
	std::lock_guard<std::mutex> lock(on_shutdown_mutex_);
	for (auto & on_shutdown_callback : on_shutdown_callbacks_) {
	on_shutdown_callback();
	}
	}
	}
	void
	rclcpp::on_shutdown(std::function<void(void)> callback)
	{
	std::lock_guard<std::mutex> lock(on_shutdown_mutex_);
	on_shutdown_callbacks_.push_back(callback);
	}

But now it means that rcl_ok()'s return value doesn't change when rclcpp::shutdown() is used, despite that really being the intention:

• https://github.com/ros2/rcl/blob/d3a734f2661a05fb1ab0746907a3477a02de8dbd/rcl/include/rcl/rcl.h#L137
• https://github.com/ros2/rcl/blob/d3a734f2661a05fb1ab0746907a3477a02de8dbd/rcl/test/rcl/test_rcl.cpp#L140-L143

[chapulina]

I added the last expectation just to make it clear that the current behaviour is that once initialized, there's no way back.

---

I don't think init -> shutdown -> init works, I tried shutting down in the end of TestUtilities.init_with_args and adding a new init test after that but an exception is thrown.

---

I'm not sure what the consequences of calling rcl_shutdown could be and whether it would be worth doing it on this PR. Whatcha think?

[wjwwood]

I don't know, but I don't want to block this. I think it's clear more work needs to be done though (after merging this).

[wjwwood]

Btw, @chapulina should this be in review? or are you still working on it?

[chapulina]

Not working on it, unless the maintainers think the shutdown issue must be fixed before getting this in.

[wjwwood]

@mikaelarguedas said it lgth, and I don't want to block it. I'll just make a follow up issue.

[wjwwood]

CI:

• Linux
• Linux-aarch64
• macOS
• Windows

[chapulina]

Fixed linter errors in f15da86

[chapulina]

• Linux
• Linux-aarch64
• macOS
• Windows

Thanks for the CI crash-course, @wjwwood !

[wjwwood]

I think this issue covers the improvements I talked about before (being able to do init-shutdown-init, etc.): #154