ROS generated code only subscribing to output channels if mentioned in trace

[MattWindsor91]

From discussion with @pefribeiro, rtcg is only generating subscribers and callbacks for topics of output channels if they are mentioned in the trace producing the test. This is bad; it means that

foo.in,foo.in

and

foo.in,bar.out,foo.in

are treated as identical traces if bar is never mentioned in the test.

This is related to #8 inasmuch as a) some of the output channels might not be named by the tests, and b) we'd need to know that the channels are output channels so as to forbid them (currently the STM JSON gets a list of inferred types for channels, but not a list of input/output directions).

[MattWindsor91]

I think the first problem arises anywhere in the templates that is ranging over .Transitions.Out (i.e. currently only considering the outbound transitions of the STM, rather than all outward channels).

Firstly, this snippet here in the STM template, generating prototypes for output callbacks:

rtcg/internal/gen/cpp/embed/templates/base/stm.cpp.tmpl

Lines 28 to 30 in a10b169

	{{- range .Transitions.Out }}
	void {{ cppCallbackName .Channel.Name }}(const {{ cppChannelMsgType .Channel.Name }} msg);
	{{- end }}

Secondly, the implementation of those prototypes (I never noticed the repetition in the function header before!):

rtcg/internal/gen/cpp/embed/templates/base/stm.cpp.tmpl

Lines 124 to 132 in a10b169

	{{- range .Transitions.Out }}
	void StateMachine::{{ cppCallbackName .Channel.Name }}(const {{ cppChannelMsgType .Channel.Name }} msg)
	{
	{{- block "out_callback" . }}
	{{- template "out_callback.cpp.tmpl" . }}
	{{- end -}}
	}
	{{- end }}

Note that this calls into a template that requires the aggregate transition set for the channel. Since the aim of the template will still be to dispatch on all the transitions involving that channel and then give up if we're not in any relevant state -- all we're doing is creating new callbacks that have no relevant states! --, this should probably remain the same, with some zero value or suchlike passed in.

And thirdly, this snippet here in the ROS main template, generating the subscribers:

rtcg/internal/gen/cpp/embed/templates/ros/main.cpp.tmpl

Lines 63 to 65 in a10b169

	{{ range .Transitions.Out }}
	auto {{ template "subscriber_name" . }} = nh.subscribe("{{ template "topic_name" . }}", 10, &StateMachine::{{ cppCallbackName .Channel.Name }}, &stm);
	{{- end }}

Of course, the question is what to replace .Transitions.Out with. We'll still need .Transitions.Out to be able to define the callback bodies, but . should probably be carrying a new list of output channels that may or may not have transitions in this test. This links back to #8 in that the channel list (and their types and directionalities) should probably be explicit rather than inferred.

[pefribeiro]

Thanks @MattWindsor91 for the details. I was under the impression that it would be sufficient to change the way in which the state machine is generated, by having additional information about all inputs/outputs (and possibly whether that should lead to stopping the machine), rather than the templates themselves? But I could be wrong. I'll take your input into consideration when I come to revisiting this.

[MattWindsor91]

@pefribeiro I forgot just how much of the logic I put in the templates...! (Wanting a powerful template language I understood better than Xtend templates is why I wrote this in Go, but at the same time I realise that the templates are somewhat reminiscent of line noise in places.)

If it was possible for a transition to throw the test off-script rather than always transitioning to another state, it might be possible to saturate the state machine by adding transitions for all outputs into the states that are accepting inputs? But that'd need a change to the templates anyway, I'd suspect, to teach them how to handle the new possibility of an off-script transition.