POETSII / Orchestrator

The Orchestrator is the configuration and run-time management system for POETS platforms.
1 stars 1 forks source link

RTS(x) in handlers rather than *readyToSend = RTS_FLAG_x #213

Closed m8pple closed 2 years ago

m8pple commented 3 years ago

This might be a temporary hack that can be ignored, but while trying to understand how to implement supervisors I noticed this:

https://github.com/POETSII/Orchestrator_examples/blob/f184e6d91aee06b3daaa96bba975f5f7e1ad6fb3/plate_heat/plate_heat_type_gals2.xml#L398-L400

I haven't seen this RTS(x) and RTSSUP(x) syntax before, and am not sure if it is a temporary thing, but it is quite a big breaking change with all previous XML.


However, I think it is generally accepted that using a bit-mask for readyToSend was a mistake. It was based on descriptions of the hardware from before any hardware existed, and anticipated optimisations that never came to pass in the real hardware (and probably only might exist in custom FPGA implementations).

In terms of analysis, verification, and model-checking it is kind of annoying to have a non-deterministic choice of which pin to send on if the application sets two of them. It can also hinder optimisation in the C++ compiler, though mostly that hasn't been an issue.

So if we want to revisit this and make the readyToSend a single choice then that is quite doable. It's quite a big breaking change, but has some advantages.

heliosfa commented 3 years ago

RTS(X) is a macro that evaluates to the “normal” RTS syntax. E.g. it becomes *readyToSend |= RTS_FLAG_x, where x has to be a valid pin name derived from the XML and RTS_FLAG_x evaluates to a bit ask for a single pin. Indeed, the normal syntax is still valid and can still be used without issue - ADB just did not like some of the pointer indirection that was involved in accessing device state, properties, etc. Etc. and asked for macros.

RTSSUP() is a special case of this macro that evaluates to a pre defined pin name used for implicit supervisor sends.

Details of the available macros of convenience can be found in 2.2.3 (page 13) of the documentation attached to #172.

m8pple commented 3 years ago

The general principle has been that convenience for humans writing code should come through generators and pre-processors, rather than new syntactic things. That's been part of the reason for longer names rather than shorter names in the code, as human typing is not so important as lack of symbol collisions.

I'm really not a big fan of macros just because they cause so many problems in generated code, so if supporting RTS(x) is necessary, then they should be real symbols. That would mean that each port name would need to be introduced as a symbol with it's short name, and RTS would become a function. Injecting a function into handler-scope is not too much of a problem I suppose, as it will get optimised away.

If andrew wants shorter typing, can he not define an m4 macro for RTS(x) and then run the graph-type through that each time he builds/runs?

Or just add

#define RTS(x) (*readyToSend=RTS_FLAG_##x)

into the shared code sections if he wants to use it? (Though it would require testing on each platform to make sure that it didn't collide with any internal details).

m8pple commented 2 years ago

This one seems to be a fait accompli now. It can be fixed in other non-Orchestrator implementations or in the graph-type, so not a real issue.