machinekit / machinekit-cnc

CNC stack split out into a separate package
Other
60 stars 36 forks source link

Design error: command serials may collide #9

Open ArcEye opened 6 years ago

ArcEye commented 6 years ago

Issue by mhaberler Fri Apr 11 17:10:32 2014 Originally opened as https://github.com/machinekit/machinekit/issues/114


Commands passed via NML typically have integer serial number for tracking status (received, being acted upon, completed, errored).

There is a fundamental design error: serial numbers are assigned on the originating side of a command. As soon as more than one command originator is active, this creates the danger of collisions, as NML has no identifier for an originator which could be used to make a submission ID unique (using a tuple (submitter, serial)).

An example for a hack around this issue is here: https://github.com/machinekit/machinekit/blob/master/src/emc/usr_intf/emcrsh.cc#L2842

problem: specify and implement a serial scheme which guarantees uniqueness.

ArcEye commented 6 years ago

Comment by sittner Tue Apr 15 12:06:28 2014


This is a real issue if you like to use HALUI in together with a normal UI like AXIS (please refer to my original bug report to LinuxCNC: http://sourceforge.net/p/emc/bugs/328/)

The patch I've submitted there is more a intermediate hot fix to ensure atomic submission ID's. A real solution to this issue will require the implementation of a publish/subscribe pattern for the result notification if a asynchronous design is required or blocking calls for command submission in case of a synchronous design. In my opinion the first one is more flexible and fits better into the current design. Michael has already done a lot of work regarding this with his ZMQ/Protobuf migration.

ArcEye commented 6 years ago

Comment by mhaberler Thu Apr 17 15:46:15 2014


I see several ways to deal with the issue, I'm not totally happy with either:

(1) any submitter first aquires a unique ID from an ID-issuing service. Guarantees global uniqueness and total ordering of ID's, no holes between ID's. There's a significant overhead associated with that.

(2) There's a single entity accepting commands and that entity issues ID's in response to the request. That guarantees ordering and no holes between ID's. De facto the global serial is a local variable of that entity. Unfortunately the single command acceptor is a significant restriction. On second thought, I exclude that option.

(3) There's a service which hands out serial number ranges, eg in blocks of say 1000 for the sake of example. Any command submitter aquires a block of ID's first, and whenever that block is depleted. Upside: total ordering, uniqueness, efficient. Downside: ID ranges might have holes (i.e. not strictly sequential). Bonus upside: the block issuing service may identify the source of a given ID in case it keeps a table {socket name, ID range}.

In tendency I'm leaning towards (3); the natural place for ID management is atomic ID allocating code code which already is in rtapi_msgd (for HAL module ID's etc). That range would be unique for an RT instance.

The only questions warranting more consideration are:

is there any downside to having 'holes' between ID's; I dont think there is; the 'equals' and 'greater than' operators are required, but those would not be affected by holes.

Restriction on ID's: they must fit with a HAL data type, i.e. for now 32bit ints. It should be straightforward to do 64bit scalars, but a UUID is out of scope (128bits required).

Question: are 'Lamport timestamps' OK as ID's? http://en.wikipedia.org/wiki/Lamport_timestamps (in other words: must the ID's support determining the order of events per originator, but not globally)

Question: sortability of ID's: certainly must hold for ID's of a single originator, but I do not see q requirement for sortability on the global level

the seminal Lamport paper: http://www.stanford.edu/class/cs240/readings/lamport.pdf

ArcEye commented 6 years ago

Comment by mhaberler Thu Apr 17 15:59:42 2014


Here's an interesting presentation on the issue: http://de.slideshare.net/davegardnerisme/unique-id-generation-in-distributed-systems , covering the theme from various angles

Also http://stackoverflow.com/questions/2671858/distributed-sequence-number-generation discusses some options.

The 'Twitter snowflake' sounds interesting as a basis: Snowflake IDs are 64-bits, half the size of a UUID Can use time as first component and remain sortable Distributed system that can survive nodes dying

The Instagram ID's would probably work, also requires 64bits: http://instagram-engineering.tumblr.com/post/10853187575/sharding-ids-at-instagram IDs consists of:

41 bits for time in milliseconds (gives us 41 years of IDs with a custom epoch) 13 bits that represent the logical shard ID 10 bits that represent an auto-incrementing sequence, modulus 1024. This means we can generate 1024 IDs, per shard, per millisecond

Another option is simpleflake: http://engineering.custommade.com/simpleflake-distributed-id-generation-for-the-lazy/

A lamport clock, but UUID-size (128bits): https://github.com/jamesgolick/lexical_uuid.erl

ArcEye commented 6 years ago

Comment by cdsteinkuehler Thu Apr 17 17:30:11 2014


On 4/17/2014 10:59 AM, Michael Haberler wrote:

The 'Twitter snowflake' sounds interesting as a basis; I would like to avoid time-synchronisation as requirement though.

Time synchronization is not required, unless you need to be able to (trivially) sort the ID's by generation time. Even without synchronized clocks, you can align the ID's if debugging by using the generator ID and a time offset for each generator.

The basic idea seems to be that each ID is composed of:

1) A guaranteed unique generator ID (which could perhaps be assigned as part of the subscribe/publish protocol?)

2) Enough bits created and assigned locally by the generator to insure no two local IDs are ever the same. Optionally, this value can include a timestamp and be constructed such that it is easy to order the events by creation time.

Charles Steinkuehler charles@steinkuehler.net

ArcEye commented 6 years ago

Comment by mhaberler Thu Apr 17 18:08:59 2014


yes, that's the idea. And time synchronization is a non-requirement, my bad. And global time is not a useful concept for event ordering in such a system anyway.

The generator ID will need to include the HAL module ID's because those are originators/consumers too, not just remote entities like UI's. So something like a remote getNextModuleId() service covers that, and ID's would unambiguously tagged as originating from a certain module (might even help debugging doing that).

As for the type of system (see http://en.wikipedia.org/wiki/Lamport_timestamps#Lamport.27s_logical_clock_in_distributed_systems) I think what we have at hand is a system with partial order between processes, suggesting this concept is the right fit

Actually I see ID's based on Lamport clocks as having upside: right now serials are int's which are happily compared with no safeguards as to origin; but comparing timestamps from different originators makes no sense by definition. But defining happened-before and equality operators on such a datatype would be able to raise an exception if timestamps of different origin are compared, which is a conceptual error. And we get a timestamp for free.

I guess next step is to do some back-of-envelope calculations to derive sensible boundaries for timestamp granularity, serial and module ID which cover the execution model for the forseeable future. I'll make it a macro and #defines just in case.

ArcEye commented 6 years ago

Comment by mhaberler Fri Apr 18 08:18:21 2014


let's start with a modified Instagram scheme. Assume 64bits timestamp size.

I'll swap the sequence number and 'shard ID' fields in size since we have different size and accuracy requirements That leaves 10bits or a maximum of 1024 for the moduleID.

Each of our IDs consists of:

This means we can generate 8192 IDs, per module, per millisecond, or every 120nS; meaning a rate of 8 million messages/second. This would be good enough for most scenarios except very tight loops (even ringbuffer operations take 100-300nS/message on my Mac), and that scenario is unlikely.

A solution could be: a generator is required to check uniqueness of an ID (this would happen only if called less than 125nS apart, so unlikely, and very low delay; that could even be done as busy-loop in an RT thread).

I think it's safe to assume that an ensemble of more than 1024 components (HAL or otherwise) talking to each other is unlikely to happen.

This timestamp is absolute for the next 41 years. Note the references to Twitter, Instagram etc cover a requirement which we dont have: these ID's are for permanent storage in databases, unambiguously tagging an item with an absolute time reference. We dont have that requirement because ID's need not be persistent across sessions.

There's one variant which might warrant consideration: use timestamps relative to startup. If we assume a maximum instance lifetime of say 1 year, that would gain us 5 bits off the timestamp.

If we put these 5bits to work in the serial (so the cut would be 36bits relative timestamp to startup, 18bits sequence, 10bits generator ID), that would mean we can generate 262 mio ID's per second or roughly a unique ID every 4nS.

Cost/benefit versus the first variant as I see it:

In fact it might not be necessary to make a hard decision on this anyway. Since we need to centrally manage the generatorID (which translates into a RPC at startup time to retrieve the next unique generator ID) we could just tack on the startup epoch, masks and shift counts as variables to this reply, making the whole decision a configuration time issue.

ArcEye commented 6 years ago

Comment by cdsteinkuehler Fri Apr 18 21:22:13 2014


On 4/18/2014 3:18 AM, Michael Haberler wrote:

There's one variant which might warrant consideration: use timestamps relative to startup. If we assume a maximum instance lifetime of say 1 year, that would gain us 5 bits off the timestamp.

If we put these 5bits to work in the serial (so the cut would be 36bits relative timestamp to startup, 18bits sequence, 10bits generator ID), that would mean we can generate 262 mio ID's per second or roughly a unique ID every 4nS.

IMHO, all of this is likely massive overkill. The systems I work with get 5-8 bits of unique transaction identifier to use. That is combined with a unique originator ID, but the result is less than 32-bits.

How long is it necessary for a transaction ID to exist? Typically, in high-speed systems like you're talking about (generating IDs on a nanosecond time-scale), the ID only needs to survive for as long as the particular transaction it's identifying.

PCI Express, for example, uses a 16-bit "generator ID" (8-bit bus ID, + 5 bit device number + 3 bit function), combined with up to 8-bits of "tag" that is assigned by the generator. That still leaves 8-bits free in a 32-bit word...exactly how "unique" do these tags need to be?

Charles Steinkuehler charles@steinkuehler.net

ArcEye commented 6 years ago

Comment by mhaberler Sat Apr 19 09:04:09 2014


just exploring the boundary cases, which includes overkill.

IMO it's not completely out of scope to consider a long-running application like some automation project with an uptime of years. I wouldnt want to have assumptions like maximum runtime == 1 year before ID rollover compiled in.

But that's not going to be a problem anyway if we go the route I laid out: generator params are distributed with the initial getGeneratorId() RPC, and those work from compiled-in defaults which could potentially be overridden by config options.

the upside of all this is - we get strong type checking on serial compares (only same generatorID is legit to compare), and timestamps for timing measurement for free (haha.. need to write this code first though).

ArcEye commented 6 years ago

Comment by cdsteinkuehler Sat Apr 19 11:30:20 2014


On 4/19/2014 4:04 AM, Michael Haberler wrote:

IMO it's not completely out of scope to consider a long-running application like some automation project with an uptime of years. I wouldnt want to have assumptions like maximum runtime == 1 year before ID rollover compiled in.

The question is do ID values have to be unique for the life of the application or just for the life of the request? IMHO regardless of how big your ID is, you will eventually exhaust the space and eventually create overlapping ID values after some (possibly quite vast) amount of time.

I see no reason ID values HAVE to be unique for longer than the existence of the transaction, which is presumably quite short. And once you can re-use ID values safely, the required number of bits shrinks dramatically.

...but I could be missing something, and it can be handy to have nice wall-clock timestamps in the IDs to make for easier debugging. I'm just trying to understand if it's really required.

I would also say it is bad design to craft a system where rollover and ID collision cause any sort of problem at all, so why not make the rollover period fairly small (say somewhere between a few seconds and an hour)?

Charles Steinkuehler charles@steinkuehler.net

ArcEye commented 6 years ago

Comment by mhaberler Sat Apr 19 11:55:22 2014


Right. Need to exclude any chances of problems.

To distinguish one ID from another, the equality operator is sufficient. But just the equality alone doesnt give you the ordering of ID's of a given originator.

Assume for a moment you tap into several components and log ID's from a birdseye view. We know that global time doesnt make sense in a distributed system (clock skew, transmission delay leading to undecidability). So all you get a bag of ID's. But if you dont have a happened-before operator on the ID's, you cant get at the temporal and hence causal ordering of events.

I think that is a fairly important property. It's laid out here: http://en.wikipedia.org/wiki/Happened-before

ArcEye commented 6 years ago

Comment by cdsteinkuehler Sat Apr 19 13:01:07 2014


On 4/19/2014 6:55 AM, Michael Haberler wrote:

To distinguish one ID from another, the equality operator is sufficient. But just equality doesnt give you the ordering of ID's of a given originator.

So the question becomes which IDs need to be compared? If it's just the active IDs for a single endpoint, the order they were queued in implicitly defines happened-before.

If we need to compare IDs in a larger time horizon that includes no-longer active IDs, the questions becomes how long a horizon and how is rollover handled.

If we need to compare IDs between multiple end-points, this gets messy real fast.

Charles Steinkuehler charles@steinkuehler.net

ArcEye commented 6 years ago

Comment by mhaberler Sat Apr 19 15:08:49 2014


right, and time sortability gives us the queuing order of a single originator (endpoints might, and do receive commands from more than one originator; eg. motion with jog-while-paused: task/ui/interp is one originator, the jog-while-paused HAL driving entity another).

making the time axis a window which can be shifted by a 'reset' type operation (forget all ID's before window start): that is an option. I think resetting time is potentially non-trivial: I guess one would need to broadcast all possible generators, make sure they flush all queues, and then collect consent from all generators before proceeding.

re ID's from different endpoints: actually that was part of my post-PhD dabblings: http://goo.gl/in4tJP and http://goo.gl/6UsXNe (executive summary); we built a language (TSL - Task Sequencing Language) to express such relations in a kind of event algebra, basically as temporal assertions. The key to linking different generators is an interaction between the generators which asserts sequencing/causality (in the TSL case an Ada rendezvous). We even had partial compile-time detection for non-causal event sequences. You could tag statements with pseudo-comments and formulate an assertion like so:

begin A => B => C end

meaning to the observer (TSL runtime), events must appear in that order or the assertion was violated; the compiler checked that A, B and C either were from the same originator, or the originators were sequenced through a rendezvous (fuzzy, it's been a while). I promise not to make TSL part of machinekit, though (what a grandiose opportunity for self-citation :)

ArcEye commented 6 years ago

Comment by cdsteinkuehler Sat Apr 19 15:22:53 2014


On 4/19/2014 10:08 AM, Michael Haberler wrote:

right, and time sortability gives us the queuing order of a single originator (endpoints might, and do receive commands from more than one originator; eg. motion with jog-while-paused: task/ui/interp is one originator, the jog-while-paused HAL driving entity another).

You're getting back to your synchronized clocks again. I thought it was decided timestamp comparisons were valid only for IDs from the same generator. If comparison across generators is required, either the problem gets much messier, or the consumer needs to timestamp the IDs when they are received.

Again, this gets back to the question of whether or not IDs need to be compared across generators and/or across consumers, and exactly what the IDs are being used for. If it's just to track messages, I don't see the need for all the complexity or ability to sort messages from the birth of the universe to entropy death with nS resolution. :)

Charles Steinkuehler charles@steinkuehler.net

ArcEye commented 6 years ago

Comment by mhaberler Sat Apr 19 22:18:28 2014


synchronized clocks: that must be a misunderstanding. A global (=synchronized) clock is a useless concept in distributed systems with asynchronous message passing, so I'm not getting back to them - time equality in such a setup is undecidable (there is lots of research into decidability of this, and this is what Lamport says which is why he advocates local clocks). It may be a useful concept in isochronous systems like the old POTS network, and electronics, but not here.

so no, those are not synchronized - they are strictly local to a generator. It is exactly like in the Lamport paper, except that the serial carries a value which can be interpreted as time, but that is just useful for display purposes. And as such, Lamport clock equality is, as you say, meaningful only within a single generator's ID's space.

Each generator's ID's define an ordering on its subset of observed events, so that gives partial orderings. Yes, linking these into a total order becomes messy: it is impossible in the general case.

What I was trying to say in response to that, with the reference to TSL, and what Lamport paper also says: under certain conditions it is possible to correlate some partial orderings, and that is possible if two events of different generators are causally related. That is for instance the case with an Ada rendezvous, or 'wait for message reception and act' in the Hewitt Actor model. In such cases you can link two such orderings into a temporal chain of events, regardless of what time-based local clocks might say (the pure Lamport clock doesnt say anything at all about absolute time, it's just a sequence number and hence relative).

There is no extra complexity involved - we are still talking a 64bit integer after all. But I think It is helpful to clearly spell out what that integer actually means and which operations can be applied to it. Just recapitulating established CS theory, a tad late though;)

ArcEye commented 6 years ago

Comment by cdsteinkuehler Sun Apr 20 03:24:02 2014


On 4/19/2014 5:18 PM, Michael Haberler wrote:

under certain conditions it is possible to correlate such partial orderings, and that is possible if two events of different generators are causally related.

That all sounds good. I'm pretty sure I finally understand what you were getting at.

I do think 64-bits is a bit much for an ID, but I don't really see how it's much worse than 32-bits for all but the slowest links (where something else can likely be implemented since there are unlikely to be thousands of generators competing at nS resolution on a 9600 baud serial link, for instance) and I think dropping to 16 bits is unnecessarily limiting.

Also, I would write the code expecting the "timestamp" values to roll-over, regardless of how ridiculously long the rollover period happens to be. Murphy says that it will happen! :)

Charles Steinkuehler charles@steinkuehler.net

ArcEye commented 6 years ago

Comment by mhaberler Mon Apr 21 04:59:17 2014


yeah, I thought about this too.

The point where most processing comes in is in the originator: generating, and tacking on to a command message, then waiting for that ID to apprear in in a response message. Typically that would be commands originating somewhere in userland and sent to RT; RT processes the command, and eventually sends a reply. In that case the ID is a pass-through value; it's not inspected or modified. So it'd be mostly userland which has to work with ID contents. The cardinality of this is roughly the number of canon commands, and that normally is just a bit more than the number of Gcode lines.

The reverse case is possible theoretically and from an API point of view: a publisher sitting right in RT and producing updates. Assuming updates are used for tracking in UI's and elsewhere, the rate is likely to be lower than 20Hz (HAL tracking is now done by haltalk - a userland proxy thread scanning remote component and group scalars for change and publishing on change - so again not RT critical).

Rollover: that could be triggered by changing the default parameters of the ID generator algorithm, which I want to make config items; like setting a short mask/number of bits for the time field.

I would love to regression-test code which includes inter-component messaging. It wouldnt hurt to overhaul the basis for regression testing in LinuxCNC, but I'm not aware of a framework which is good at distributed setups.

ArcEye commented 6 years ago

Comment by mhaberler Thu Apr 24 04:52:04 2014


beginnings of requirements and cleanup work needed:

The 'machineflake' will consist of a local timestamp, a serial, and a generator ID. While boundaries may be configurable, the generator ID needs to run from 0..max where max fits within the bits allocated for the generator ID.

For minimum confusion, it is best to reuse the rtapi module ID (which doubles as HAL component ID). However, if this ID is used, it needs to be contiguous within the 0..max range, and this is currently not the case for rt-preempt/posix (RT comp ID's start at 32768+ to distinguish them from userland modules which start at 1+).

So there's some minor unification work needed in RTAPI to prepare for such an ID space:

As a positive side effect, a flake is tagged as RT- or non-RT originated.

decision item: reuse module ID's, or not? non-reuse might imply ID space exhaustion at some point. Probably best to employ an LRU scheme for reallocation.

prepartory step: unify RTAPI module ID's to be contiguous for RT, and non-RT ID's be unique and outside the RT ID range.

ArcEye commented 6 years ago

Comment by mhaberler Tue Apr 29 04:01:31 2014


I've spent too much time on this, so for now I'm standing back for a simple serial/generatorID scheme (32bits each) so the concept can be worked, but the serial cannot be interpreted as a timestamp for now.

One of the issues was: find hires timing sources which are consistent across userland and RT, for all thread flavors, and which can be interpreted as wall clock time; otherwise one gets into correlating and adjusting for the difference of two different timer sources. Also, for reducing the number of bits required, a custom epoch is needed, so this gets messy very quickly.The mailing lists helped, the summary so far is:

RTIME rt_get_cpu_time_ns(void)
{
      // llimd is a fast 64/32 multiply/divide operation
      return llimd(rdtsc(), 1000000000, tuned.cpu_freq);
}

This code can be replicated on userland/RT x86 hosts but needs an offset calibration step for wall clock time.

Summary: likely a new RTAPI primitive (rtapi_get_timestamp() or so) is needed, as well as flavor-dependent userland code (ULAPI). Timestamps generated on non-RT hosts probably should use CLOCK_REALTIME.

Related: http://man7.org/linux/man-pages/man2/clock_gettime.2.html

ArcEye commented 6 years ago

Comment by cdsteinkuehler Tue Apr 29 10:19:34 2014


On 4/28/2014 11:01 PM, Michael Haberler wrote:

I've spent too much time on this, so for now I'm standing back for a simple serial/generatorID scheme (32bits each) so the concept can be worked, but the serial cannot be interpreted as a timestamp for now.

I'm pretty sure I said somewhere above that timestamps were optional. :)

I also recommend making it VERY easy to adjust the 32-bit size of your serial number. We will want to be able to drop that to something like 8 bits or less to test for problems with roll-over. Honestly, I'd initially develop using a small serial number field to avoid any roll-over issues at the outset, and increase the size when you get to the point where you are rapidly generating messages and need more ID space.

Charles Steinkuehler charles@steinkuehler.net

ArcEye commented 6 years ago

Comment by mhaberler Tue Apr 29 10:25:58 2014


makes sense, but in the minimum the serial space must exceed the length of the largest queue to be handled, or you'll have a wraparound event before the queue is filled, meaning you can't track entries properly anymore

eg the tp queue has DEFAULT_TC_QUEUE_SIZE 2000

ArcEye commented 6 years ago

Comment by mhaberler Wed Jul 8 05:50:14 2015


this is an interesting & practical paper on timestamps in distributed systems .

ArcEye commented 6 years ago

Comment by mhaberler Thu Oct 15 08:30:43 2015


good solution, but likely overkill: How to Have your Causality and Wall Clocks, Too by Jon Moore

ArcEye commented 6 years ago

Comment by machinekoder Sat Oct 17 03:50:04 2015


Thats pretty similar to how the time synchronization of FlexRay works.

ArcEye commented 6 years ago

Comment by mhaberler Wed Dec 9 09:21:01 2015


yesterday's talk on task showed significant interest in fixing the serial number collision problem; now that the talk established some common understanding how task and UI's interact we might refocus on the problem - it is in fact more involved than just avoiding serial collisions.

this thread diverted a bit from the original problem to the mechanics of defining serials, so let me recap what the issues actually are:

  1. serial numbers may collide as they are created by task clients without coordination.
  2. serial numbers in the current code base need to be unique but not monotonically increasing
  3. the actual problem cannot be fixed by a guaranteed-to-be-unique serial alone
  4. it probably cannot be fixed as long as we have NML/RCS as middleware.

re (1): to understand the issue, look at the code in emcmodule.cc which injects commands into task:

re (2): I mentioned I was not sure about serial numbers being assumed to be monotonically increasing - this is not the case. To see why, one can grep for all uses of echo_serial_number in the code base and check which comparison operators are applied. This command does that: grep -r echo_serial_number src/emc/|egrep -e '(>|<)' - the hits contain no use of < or > operators. However, the patch by @sittner referenced in linuxcnc bug 328 would change that to monotonically increasing serials are assumed . A variant thereof has found its way into the linuxcnc code base, which partially fixes the problem.

re (3): unique serials are not sufficient. A second problem is the semantics of the NML status channel - it does not provide queued communication but rather has a last update counts semantics. @sittner describes the issue here: if several clients are injecting commands, the EMC_STAT structure as provided by the status channel only shows the last update, a previous update which - for whatever reasons, like random delays - has not been received in time by the originating client is lost as it is overwritten by a faster second client. So this is a race condition between several clients updating their view of EMC_STAT and task updating same.

To summarize, the "global shared buffer update" as provided by the NML status channel does not provide the semantics necessary to funnel individual acknowledgements to their respective originators.

What would be needed is an RPC-style per-client bidirectional communications pipe which has the following property:

  1. a command injected is tagged with a unique originator id.
  2. task must be able to monitor an arbitrary number of such per-client pipes.
  3. Responses to a request coming in on a particular pipe must be directed only to the originating client, based on the originator id from (1).
  4. These pipes must support queued semantics - no update may get lost either way.

(1) assures task cannot get confused where a command came from. (2) assures several UI's may work in parallel. (3) and (4) fix the lost update problem.

I note that NML does not support such an RPC-style pattern. I have tried in the past, and it is essentially not doable for the lack of queued operations, private pipes between using entities, and a lack of a originator ID mechanism which aids the identification of the originator and the routing of the replies. So frankly this might not be doable at all properly as long as we use NML.

Note that zeroMQ has all what is needed - a DEALER socket in a client and a ROUTER socket in task does provide the required semantics. zeroMQ also takes care of unique client ID's so the tuple of (client ID, serial) would still be unique. That said, it probably would still be desirable to have unique ID's because it aids tracking messages as the flow through the system, and they would be more HAL-friendly than a tuple.

So, if one were to go about to fix this design flaw once and for all:

On a migration strategy:

I actually did some work addressing the last item but that was never merged. However, it worked as advertised, and mixing zeroMQ transport and NML message contents was not an issue.

The problem is rather isolated - the command input really. What we need is some impact analysis - need to identify all code which injects command into task, and see if they all are needed.

$ grep -rl echo_serial_number emc/
emc/usr_intf/emcsh.cc
emc/usr_intf/emclcd.cc
emc/usr_intf/emccontroller.cc
emc/usr_intf/shcom.cc
emc/usr_intf/keystick.cc
emc/usr_intf/emcrsh.cc
emc/usr_intf/axis/extensions/emcmodule.cc
emc/usr_intf/halui.cc
emc/usr_intf/xemc.cc
emc/usr_intf/schedrmt.cc
emc/iotask/ioControl.cc
emc/iotask/ioControl_v2.cc
emc/task/emctaskmain.cc
emc/task/taskintf.cc
emc/task/taskclass.cc
emc/task/iotaskintf.cc

Out of these, emcsh emclcd keystick emcrsh emcmodule halui xemc schedrmt are bona-fide UI's so those would have to migrate in concert; some might have to go anyway like schedrmt

thinking of it - there might be a way to do this piecemeal:

see also the list post

ArcEye commented 6 years ago

Comment by ArcEye Wed Dec 9 11:15:34 2015


I actually did some work addressing the last item but that was never merged. However, it worked as advertised, and mixing zeroMQ transport and NML message contents was not an issue.

Can you point to this as a starter, in your repo or wherever.

ArcEye commented 6 years ago

Comment by mhaberler Wed Dec 9 13:19:14 2015


this is the (branches - for-sascha, and the underlying zmq-submit-task.commands) http://git.mah.priv.at/gitweb?p=emc2-dev.git;a=shortlog;h=refs/heads/for-sascha

this was about using the 'cheesestand ticket' analogy I referred to, so that is just one option

when reading this branch, I suggest to concentrate on the diffs of src/emc/task/emctaskmain.cc ("server" side) and src/emc/user_intf/axis/extensions/emcmodule.cc ("client" side)

I did some writeup on migration back then, but that was about taking too big a bite at one step (including protobuf and whatnot) - take with a big lump of salt

Also since @einstine909 is now on the canon case, we will have several options downstream

But IMO just adding a zeroMQ path in parallel to the NML input channel looks perfectly doable and pretty well isolated

also see the standalone command injection demo

not sure all this still works, ping me before doing anything more than reading, then I'll rebase onto mk first if you want

ArcEye commented 6 years ago

Comment by ArcEye Wed Dec 9 13:49:21 2015


I suspected you had already 'invented the wheel', just needed a tyre change :smile: Won't be able to look properly for a few days.

emcmodule.cc doesn't do anything for me, but I thought of implementing the server in emctask as a parallel command insertion route and modifying a copy of my C++ access libraries to pump their commands by that route.

If that worked, someone would just need to convert perfectly good C++ code into parsel-tongue bindings, to get the python equivalent :snake:

ArcEye commented 6 years ago

Comment by mhaberler Wed Dec 9 13:56:35 2015


oh, let me have a look in the evening, I'll clean up and rebase, and review what I actually did (and what actually worked)

it could be just that the standalone injection demo was what worked, and I had not touched emcmodule yet at all

on the parsel tongue thingie replacing 'import linuxcnc' : almost. zeroMQ would be just fine as handled by 'import zmq'. The bummer is: as long as we use NML we need to remain in C++ land. As soon as the message content is encoded in protobuf its all Python (and downhill from there ;)

ArcEye commented 6 years ago

Comment by bobvanderlinden Thu Dec 10 14:36:29 2015


@mhaberler Saw your talk yesterday, thanks for all the information!

Avoiding the duplicate serials globally is a good idea. Regarding using ZeroMQ, does that mean milltask is being changed to have something like http://zguide.zeromq.org/page:all#Shared-Queue-DEALER-and-ROUTER-sockets (Figure 16)?

ArcEye commented 6 years ago

Comment by klerman Thu Dec 10 15:34:09 2015


@mhaberler

Might it be possible to write a wrapper around zeromq to make it have the NML interface and just drop it in everywhere? Then migrate code to use zeromq directly as appropriate (where additional functionality is needed)?

Ken

ArcEye commented 6 years ago

SYNOPSIS OF NEXT 18 comments

@klerman commented on 10 Dec 2015

@mhaberler

Might it be possible to write a wrapper around zeromq to make it have the NML interface and just drop it in everywhere? Then migrate code to use zeromq directly as appropriate (where additional functionality is needed)?

Ken @mhaberler Member mhaberler commented on 11 Dec 2015

@bobvanderlinden yes, the DEALER (client) - ROUTER (task) combination is the best match and is what I used in my experiment back then for command injection

for the back channel (task->clients) there are two options:

just use the same socket for replies - reliable but reply only visible to client; pretty much an RPC pattern
use a PUB socket for general task status reporting (including all of io/motion/aux/ etc status blobs, reported as distinct topics eventually), and use that as return channel for command acks (received/in progress/done as yet one more topic)

the latter has the advantage of all status going through a single channel so no proliferation of special-purpose channels (for instance the error channel is another candidate to collapse into general status publishing); and it is also visibly to any interested party (debugging, UI's)

the downside is: there are really two kinds of status information. One is: last value counts - here you can afford to loose an update, since the next update will correct things. The other kind is: every value counts - and here one cannot afford to loose an update; eg if serial matching is employed and updates might be lost, then the client would deadlock waiting for a lost update.

The likelihood of that happening is extremely low and there might be ways around it, but it is a concern at least in theory; the drop might happen sender side (task) under memory pressure. Fact is - most of the other content in EMC_STAT which will be funneled through a PUB socket is 'last value counts' so no issue there.

I have not made up my mind for good which is the preferable pattern; it might also be possible to reply on both channels (PU/SUB for the casual observer, and ROUTER->DEALER reply channel for the command injector) - actually I'd appreciate a comment by @Strahlex (what do you think?)

now for the experiment back then which I'm currently rebasing onto machinekit master, for the back channel (task acks -> client) I used a PUB/SUB pattern and that worked fine

I dropped client side serials and always have task assign ID's so no collision possible

this is the experiment's flow - client->listener goes via DEALER/ROUTER, listener->client goes via PUB/SUB:

http://static.mah.priv.at/public/ticket-pattern.png @mhaberler Member mhaberler commented on 11 Dec 2015

@klerman - wrappers usually sound like great migration aids

the reasons why I think they do not significantly aid migration in this case are: the "NML interface" has two parts: the message encoding, and the command/status/error channels.

Fact is: the channel abstraction is broken - status kind of works, but the error channel is dismal and command injection as deployed is broken beyond repair, even if you made serials unique (see further down).

So what do you wrap - water down zeroMQ flows to the extent they match that channel pattern in all brokenness, to get around the "need to change source" issue? The result will be functionally not better than what we have. You cannot fix the serial issue by clever macros - the flows are defunct.

The other migration issue which cannot be fixed by the clever wrapper: there are no non-C++ bindings to NML encoding. Real boat anchor, say for rewriting task in Python.

I think the better strategy is:

introduce zeroMQ patterns for interaction piecemeal, for instance for now for the command channel.
do that in parallel to the legacy interaction, which is clearly possible.
initially use NML in the new patterns, then move on to using protobuf encoding for that channel.
make emcmodule.cc to work with both - default to new style, and have it fall back to the legacy method in case of problems, eg through an environment variable, for the sake of testing and big isolation.
move that partial migration into master. Give it a shakeout by users.
80/20: always concentrate on the linuxcnc module (emcodule.cc) because that covers most of the UI needs. I frankly do not care about keystick or mini using zeroMQ right away (they will keep working on the old channels).

There is another fundamental issue with multiple UI's injecting commands: while it might be possible to inject commands into task from several UI's, it is unclear how they should be processed - sequenced, or in parallel, or sequenced plus priority.

Example:

Run an MDI command which takes some time, like a feed move. A second UI comes in and also executes a feed move. One would think - sequenced behavior of treatment of commands is the right thing to do - run move 2 after move 1 completes.

Now replace the second move by an abort command. The result is not what you want: the abort treated after move 1 completes, by which time the tool might have banged into something.

It looks task was not designed with this scenario in mind; people kind of stumbled onto the fact that one could inject commands from parallel as the channel API permits that, but not much thought was given what the execution semantics should be and if this is actually a safe thing to do.

A possible solution to that is to distinguish commands into "queueable" and immediate commands, the latter taking effect immediately (and possibly affecting the queue, eg by flushing it, like in the case of abort).

---- update ----

reading the log & source I think I now understand how task distinguishes "immediate" and "queued" commands and how that pertains to the command input channel:

commands are received and acted upon pretty much every cycle, so acking the reception happens immediately
for my MDI command example - assume one or several moves are commanded: those go through a call to the interpreter, and that in turn populates the canon queue (interp_list) with those move commands. So that is where they sit until executed. However, the input channel never blocks.
so assume the move is executing, and an abort message comes in: this does stop any motion in progress and flushes the interp_list.
the flow of status updates (which command is in which status, and it's id) is pretty much independent of the above, except for clients doing serial number matching

@ArcEye Member ArcEye commented on 14 Dec 2015

Now looked at your original code - you have pretty much already done it. smile

I'll wait for your merge into a current branch, it currently fails to build with protobuf errors that I recall from quite a while back. ( protobuf/generated/types.pb.h not being created etc - not sure if current libs are compatible ) @mhaberler Member mhaberler commented on 14 Dec 2015

(trying to really understand what I was talking about last time ;) Some notes on the task state machine

The task execution state ('execState') is one of the following:

// types for EMC_TASK execState enum EMC_TASK_EXEC_ENUM { EMC_TASK_EXEC_ERROR = 1, EMC_TASK_EXEC_DONE = 2, EMC_TASK_EXEC_WAITING_FOR_MOTION = 3, EMC_TASK_EXEC_WAITING_FOR_MOTION_QUEUE = 4, EMC_TASK_EXEC_WAITING_FOR_IO = 5, EMC_TASK_EXEC_WAITING_FOR_MOTION_AND_IO = 7, EMC_TASK_EXEC_WAITING_FOR_DELAY = 8, EMC_TASK_EXEC_WAITING_FOR_SYSTEM_CMD = 9, EMC_TASK_EXEC_WAITING_FOR_SPINDLE_ORIENTED = 10 };

The starting, and completion state is of the task state machine is EXEC_DONE.

execState is relevant only for executing commands off the canon queue (interplist) - a new command will be fetched ONLY if the state is EXEC_DONE.

execState is NOT relevant for 'immediate' commands (those passed by a UI through the input channel).

However, several immediate commands cause canon commands to be emitted, either by calling upon the interpreter, or by directly queuing commands through emcTaskQueueCommand.

Each cycle of task executes emcTaskPlan() and emcTaskExecute() in turn.

emcTaskPlan() reads any new command (emcTaskCommand) from the input channel, and decides what to do with it based on the mode (manual, auto, mdi) or state (estop, on) of the machine. Many of the commands just go out immediately to the subsystems (motion and IO). In auto mode, the interpreter is called and as a result the interp_list is appended with NML commands. emcTaskCommand is the 'pending command' (currently operated upon); when done, it is zeroed.

emcTaskExecute() executes a big switch on execState. if the status is EXEC_DONE, it gets the next item off the interp_list, and sets execState to the preconditions for that. These preconditions include waiting for motion, waiting for IO, etc. Once they are satisfied, it issues the command, sets execState to the postconditions, and zeroes the pending command. Once the postconditions are satisfied, it gets the next item off the interp_list, and so on.

preconditions and postconditions are only looked at in conjunction with commands on the interp_list. Immediate commands won't have any pre- or postconditions associated with them looked at.

Single-stepping is handled in checkPreconditions() as the first condition. If we're in single-stepping mode, as indicated by the variable 'stepping', we set the state to waiting-for-step. This polls on the variable 'steppingWait' which is reset to zero when a step command is received, and set to one when the command is issued.

Any execution error sets execState to EXEC_ERROR. On the next emcTaskExecute(), in the EXEC_ERROR state all queues are flushed, motion, interpreter,spindle, iocontrol, stepping etc is stopped and execState is set to EXEC_DONE. A sync command is queued so next cycle the interpreter is synced.

On data structures used (a tad confusing):

emcCommand points to the current command in the input channel. That pointer is never NULL. Do determine if a command is pending in the input channel, emcTaskPlan() compares the the emcCommand->serial_number with emcStatus->echo_serial_number. When the command is completed, echo_serial_number is updated to serial_number which finishes processing of the current emcCommand.
emcTaskCommand is used only for commands pulled off the interplist, and can be NULL if there is currently nothing to execute off the interplist.

The distinction between emcStatus->task.echo_serial_number and emcStatus->echo_serial_number still confuses me, not sure why both are needed

grepping through the source it seems task.echo_serial_number is in fact unused, the emcmodule.cc code does not even export it and it seems unreferenced other than in emcsh.cc as the Tcl "emc_task_command_number" variable; for that usage I actually assume it is in error - which seems not to matter since emcStatus->task.echo_serial_number and emcStatus->echo_serial_number are set to identical values. Moreover, "emc_task_command_number" is only used in tkemc so it seems the other UI's can live perfectly happy without it; no big deal since the Tcl/Tk UI's are heading for the chop anyway:

mah@jessie64:~/machinekit-master/src$ grep -r task.echo_serial . ./emc/usr_intf/emcsh.cc: commandnumber = Tcl_NewIntObj(emcStatus->task.echo_serial_number); ./emc/usr_intf/xemc.cc: emcStatus->task.echo_serial_number, ./emc/task/emctaskmain.cc: emcStatus->task.echo_serial_number = emcCommand->serial_number; ./machinetalk/mkwrapper/mkwrapper.py: self.status.task.echo_serial_number = 0

@mhaberler Member mhaberler commented on 16 Dec 2015

I've rebased the 'inject task commands via zeroMQ' proof-of-concept onto machinekit master, it is here: https://github.com/mhaberler/machinekit/commits/task-zmq-cmd-channel

this implements this ticket pattern - you might want to read up on the tracker issue and the discussion with Sascha back then to get the idea

have a look at src/emc/task/README.zeromq to get the gist @ArcEye Member ArcEye commented on 16 Dec 2015

It certainly works.

If you turn on ZDEBUG you can trace the Axis calls that use emcmodule.cc ( eg machine on ) and see them being passed through the zmq mechanism

[011] emcmod25012 [029] 08139A0118F90100000000000018000000000000000000000003000000 task: received EMC_TASK_SET_STATE ticket 24 15-12-16 12:28:08 task: reply message:

[011] emcmod25012 [013] 08900372085518000000A00104 emcTaskPlan new command EMC_TASK_SET_STATE, 24/0 Issuing EMC_TASK_SET_STATE -- (+505,+24, +24, +3,) emcmod25012: got ticket = 24 NML_INTERP_LIST::append(nml_msg_ptr{size=24,type=EMC_TASK_PLAN_SYNCH}) : list_size=1, line_number=243 emcTaskPlanSynch() returned 0 ----- current state: EXEC_DONE: precondition EXEC_WAITING_FOR_MOTION_AND_IO for command EMC_TASK_PLAN_SYNCH NML_INTERP_LIST::append(nml_msg_ptr{size=24,type=EMC_TASK_PLAN_SYNCH}) : list_size=1, line_number=243 task: update ticket 24 status=RCS_RECEIVED->RCS_EXEC to origin=emcmod25012 listener emcmod25012: cticket = 24 status=2 ----- current state: EXEC_DONE: precondition EXEC_WAITING_FOR_MOTION_AND_IO for command EMC_TASK_PLAN_SYNCH ----- transition (io+motion done) EXEC_WAITING_FOR_MOTION_AND_IO -> EXEC_DONE Issuing EMC_TASK_PLAN_SYNCH -- (+516,+24, +0,) emcTaskPlanSynch() returned 0 ----- current state: EXEC_DONE: postcondition EXEC_DONE for command EMC_TASK_PLAN_SYNCH task: update ticket 24 status=RCS_EXEC->RCS_DONE to origin=emcmod25012 listener emcmod25012: cticket = 24 status=1

Will take a while to get my head around it.

In the interim, the true awfulness of the alleged C++ legacy code in task, hits you in the face.

Looks like someone thought it would be a 'clever' idea to type everything as class EMC_XXX : public RCS_CMD_MSG........ so that you could use inheritance, instantiate it and use its methods / access it data members but that is as far as any C++ code extends. The .cc files contain just C functions and are littered with global variables-1 @mhaberler Member mhaberler commented on 18 Dec 2015

I discussed with @Strahlex patterns yesterday and I'll use the "commands and status updates both via the dealer/router channel" scheme, working on it

that said, it occurred to me that there is a design flaw with respect to "task execStatus"

as far as a UI is concerned, it is not particularly interesting what the "task execStatus" is, whatever that means - it is more interesting to see the status of a particular command a UI just posted

now as long as there is only one input channel (NIST design singletonitis again) it does not matter where that single status value lives; as soon as there can be several UI's injecting commands there is no way to express the status of a particular command, which is where task.execStatus gets pretty useless and status updates of individual commands go haywire, which I guess is part of the problems observed

what is needed therefore is a representation in task of all commands currently pending, not just the last one, and a means to associate status with a particular command - only then all of them can be properly closed off in case of say an EMC_TASK_ABORT and with the right vehicle (update echo_serial_number? report status on dealer socket? one needs the origin to do the right thing). With only the last command being represented one is bound to miss something eventually.

what is also lacking is a basic liveness test of the UI/task interaction: run axis, kill -9 milltask and notice the UI has no clue that milltask went away - it just freezes. So a periodic ping scheme is on order.

that aside, command injection done properly with zeroMQ will be a great wedge to drive in along the 'dump NML' caravan path, since it can live perfectly fine in parallel with legacy mode and switched as needed in case of problems

so as an outline what I'll do on the issue:

I'll finish the prototype ticket pattern scheme so emcmodule.cc works with command injection and command execution status just over a dealer socket (no separate status publisher/subscriber pair)
once this works again, emc/task gets a brushup: the monster source files will be split up into per-function files and includes to make coupling explicit, plus fixing some of the most blatant uses of globals by replacing them with parameters - this refactor will happen before any zeroMQ stuff is applied and should go straight into master since no functional change
re-applying the zeroMQ injection will go into a development branch thereafter
we need to run the task cycle off a czmq event loop (zloop), the reason being: we need to announce the zeromq sockets via mDNS and that avahi integration requires a zloop inner loop - nothing major, mostly a rehsuffle
next is the 'pending command representation' issue - likely a list or dict of structs recording origin, command and respective status
then zmqcmd.cc and emcmodule.cc fixed up to work with this scheme
then we can look into merging

@ArcEye Member ArcEye commented on 19 Dec 2015

next is the 'pending command representation' issue - likely a list or dict of structs recording origin, command and respective status

This is my starting point for a linked list, using the utlist headers (http://troydhanson.github.io/uthash/utlist.html )

https://github.com/ArcEye/machinekit/commits/task-zmq-cmd-channel

Currently just creates a list and prints out contents at task exit. (the status just shows the initial value set - not updated yet )

14 elements in cmd_list From emcmod27455 - Ticket No. 2 - Command EMC_TASK_PLAN_SET_BLOCK_DELETE - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 3 - Command EMC_TASK_PLAN_SET_OPTIONAL_STOP - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 4 - Command EMC_TASK_SET_MODE - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 5 - Command EMC_TASK_SET_MODE - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 6 - Command EMC_TASK_PLAN_OPEN - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 7 - Command EMC_TRAJ_SET_SCALE - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 8 - Command EMC_TRAJ_SET_SPINDLE_SCALE - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 9 - Command EMC_TRAJ_SET_MAX_VELOCITY - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 10 - Command EMC_TRAJ_SET_SPINDLE_SCALE - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 11 - Command EMC_TRAJ_SET_SCALE - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 12 - Command EMC_TASK_SET_STATE - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 13 - Command EMC_TASK_SET_STATE - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 14 - Command EMC_TASK_SET_MODE - Status = RCS_RECEIVED From emcmod27455 - Ticket No. 15 - Command EMC_TRAJ_SET_TELEOP_ENABLE - Status = RCS_RECEIVED

The other option is using a hash based on the ticket number, which would be a good key as it should be unique (whole point of all this laughing ) but either would work. Thinking about it, a hash like that, would make it necessary to know the ticket number - probably better iterating through a list, to search for what ever field you choose to.

Once I get it fully connected, I'll abstract it into functions within zmqsuppport.cc probably. @ArcEye Member ArcEye commented on 19 Dec 2015

Now moved into zmqsupport.cc and state updated in the publish_ticket_update() loop

Full output has updated states of tickets.

14 elements in cmd_list From emcmod20453 - Ticket No. 2 - Command EMC_TASK_PLAN_SET_BLOCK_DELETE - Status = RCS_DONE From emcmod20453 - Ticket No. 3 - Command EMC_TASK_PLAN_SET_OPTIONAL_STOP - Status = RCS_DONE From emcmod20453 - Ticket No. 4 - Command EMC_TASK_SET_MODE - Status = RCS_DONE From emcmod20453 - Ticket No. 5 - Command EMC_TASK_SET_MODE - Status = RCS_DONE From emcmod20453 - Ticket No. 6 - Command EMC_TASK_PLAN_OPEN - Status = RCS_DONE From emcmod20453 - Ticket No. 7 - Command EMC_TRAJ_SET_SCALE - Status = RCS_DONE From emcmod20453 - Ticket No. 8 - Command EMC_TRAJ_SET_SPINDLE_SCALE - Status = RCS_DONE From emcmod20453 - Ticket No. 9 - Command EMC_TRAJ_SET_MAX_VELOCITY - Status = RCS_DONE From emcmod20453 - Ticket No. 10 - Command EMC_TRAJ_SET_SPINDLE_SCALE - Status = RCS_DONE From emcmod20453 - Ticket No. 11 - Command EMC_TRAJ_SET_SCALE - Status = RCS_DONE From emcmod20453 - Ticket No. 12 - Command EMC_TASK_SET_STATE - Status = RCS_DONE From emcmod20453 - Ticket No. 13 - Command EMC_TASK_SET_STATE - Status = RCS_EXEC From emcmod20453 - Ticket No. 14 - Command EMC_TASK_SET_MODE - Status = RCS_DONE From emcmod20453 - Ticket No. 15 - Command EMC_TRAJ_SET_TELEOP_ENABLE - Status = RCS_DONE

Needs a polish but seems entirely workable. Repo updated

If you can work with this method, I will expand upon it. @mhaberler Member mhaberler commented on 19 Dec 2015

hold those horses ;) not clear yet what the datastructure needed is since we have not fully thought through the problem yet; thinking aloud now:

Acknowledgments - legacy NML. Simple if just one UI legacy channel. Just sets the echo_serial_number and status to whatever is being completed and write the status buffer. Broken if more than one UI connected, likely since serials go haywire in EMC_STAT.
Acks - zeroMQ and NML in parallel: different ack mechanisms; on zeromq/ticket scheme, immediately replies with assigned ticket/RCS_RECEIVED then RCS_EXEC/RCS_DONE/RCS_ERROR acks as results come in. Note that we need to record the nature and origin of the current command so we can do the right thing according to channel - bump echo_serial.. on NML or reply on the right dealer channel (assuming the general case - there are several zeromq UI's).
A pending command is aborted by either input channel receiving an EMC_TASK_ABORT. Now both need acks (they could even come from different channels). That is currently not possible since we only record the last command and origin (which would be the abort, so the pending command being aborted really goes unacknowledged; we do not want that to avoid a UI deadlocking waiting for a serial/ticket in the ack which never shows up.

To address the above three, we need to record the currently pending command (but not the whole sequence) - and in case the abort comes in on the same channel as the pending command, the abort origin/type as well. On an abort we iterate the pending commands in ascending serial/ticket per channel, close those off, then ack the abort.

On top of this we have the current have-baked "MDI Queuing" behavior (looong thread back then on emc-developers). Here is the scenario:

example - you fire off two MDI commands without waiting for RCS_DONE after the first one.
what happens is - both MDI commands are passed to the interpreter right away, and populate the canon queue (the mythical "MDI queue" is in fact the canon queue).
this just happens to work if the MDI commands have NO queue buster ops. If the first one has a queue buster, but the second MDI has not, the execution of the second MDI happily tramples over the interpreter state of the first command (which might still be waiting for the queue to finish, execute the queue buster and continue
the whole thing is a bug redressed as a feature. it happens because task blindly calls the interpreter read/exec functions without regard what is still going on.

Just saying we need to keep this one in mind. The more I think of it though - I guess #106 will take care of this as the interpreter becomes master of its main loop, and will be fed through a queue (another dealer/router pair) between task and interpreter - it will just not start executing the second MDI before the first one finishes. So then we'll have a proper queueing behavior of the interpreter as a result of closing off #106. Once the interpreter dequeues a command she'll ack the command, and task can send an RCS_EXEC update to the originator.

Anyway - the simplest and most general tracking mechanism is likely a list. This is how it would work:

every command coming in is recorded in this list, by appending, with: channel, serial or ticket, status.
status change of a ticket or serial: search list for match of serial/ticket and channel. Record status; or if status is RCS_DONE, delete the list item after sending update in proper channel/mechanism.
abort: add the abort command. Execute it. when done, complete the whole list of pending commands starting oldest first, acking with proper channel/mechanism.

To verify we got things right, a periodic scan on task idle must meet an empty command list. If not, we must have missed an ack which is an error. @mhaberler Member mhaberler commented on 19 Dec 2015

@ArcEye your code seems pretty much there; we do need NML channel commands just alike

If I have a say I would go for a std::list instead of the C macros you proposed - much harder to screw up ;)

not sure how to handle the origin ID in case of several NML UI's present - there is no originator id

but then we could skip the issue and assume the multiple-UI case is fixed ONLY in the zeromq scenario, and for NML only one channel is supported - no point in saving the whales now @ArcEye Member ArcEye commented on 20 Dec 2015

If I have a say I would go for a std::list instead of the C macros you proposed - much harder to screw up ;)

Implemented: ArcEye/machinekit@2277eb4 It is just a place holder for now really, it is the connections and usages that need the thought.

not sure how to handle the origin ID in case of several NML UI's present - there is no originator id
but then we could skip the issue and assume the multiple-UI case is fixed ONLY in the zeromq scenario, and for NML only one channel is supported - no point in saving the whales now

I think that is the only sane way to go. The longer term aim is to get rid of NML, if the emcmodule.cc uses zmq to transmit the commands, that takes care of 90% of cases via Axis and gladevcp panels, which both use the python bindings. Not sure about Alex's remote stuff though.

I am sure I could break it using my C++ libraries, but since I pretty much only do single user, single UI, I would have to create an extra active panel that uses NML commands directly to do that. @mhaberler Member mhaberler commented on 20 Dec 2015

Alex's mkwrapper sits on top of the current emcmodule and is written in Python, so no impact

Post having gotten rid of NML, all of emcmodule.cc can be rewritten in plain Python (using import zeromq protobuf etc), and we can get rid of the polling and use proper event notification @ArcEye Member ArcEye commented on 21 Dec 2015

status change of a ticket or serial: search list for match of serial/ticket and channel. Record status; or if status is RCS_DONE, delete the list item after sending update in proper channel/mechanism.

Now implemented in the same publish_ticket_update() section that updates the state If state update is RCS_DONE, deletes that element.

Also added a check, just for debug purposes, at the end of the main() loop in task, that checks if EMC_TASK_INTERP_IDLE is set, and if so whether there are uncompleted commands. To save reams of output, only repeats if the uncompleted commands number rises.

WARNING - Task is idle, but command list contains 3 uncompleted commands 3 elements in cmd_list From emcmod13357 - Ticket No. 13 - Command EMC_TASK_SET_STATE - Status = RCS_EXEC From emcmod13357 - Ticket No. 18 - Command EMC_TASK_PLAN_RUN - Status = RCS_EXEC From emcmod13357 - Ticket No. 20 - Command EMC_TASK_SET_MODE - Status = RCS_EXEC

In theory this should never get above a count of 1 in normal use, (command to be executed on next iteration of the loop) but above is from abort midway through running program.

At close down, shows still 2 commands outstanding

calling previous SIGTERM handler 2 elements in cmd_list From emcmod13357 - Ticket No. 13 - Command EMC_TASK_SET_STATE - Status = RCS_EXEC From emcmod13357 - Ticket No. 18 - Command EMC_TASK_PLAN_RUN - Status = RCS_EXEC Cleanup done

ArcEye@4be60a8 @mhaberler Member mhaberler commented on 24 Jun 2016

taking the liberty to record @koppi's suggestion:

Von: Jakob Flierl jakob.flierl@gmail.com
Betreff: Aw: [Machinekit] Free CAD / CAM / CNC combination | OpenJSCAD + Kiri:Moto + Machinekit
Datum: 24. Juni 2016 um 00:21:31 MESZ
An: Michael Haberler mail17@mah.priv.at
Kopie: Machinekit machinekit@googlegroups.com

https://github.com/willemt/ticketd is a C implementation of a
distributed durable unique 64bit ID server on top of
https://github.com/willemt/raft which has it's theoretical roots on
Paxos but is more practical for implementation ( – most Paxos protocol
implementations in fact look more similar to the
https://en.wikipedia.org/wiki/Raft_(computer_science) protocol).

This could be useful, if you want to synchronize IDs among tasks of
two distributed Machinekit instances. But for the simple use-case
above, the techinque with allocating serial ranges is probably a good
choice as a trade-off between implementation complexity and feature
completeness.