M67 commands works only for outputs 0-31

[ArcEye]

Issue by machinekoder Thu Jan 29 13:46:25 2015 Originally opened as https://github.com/machinekit/machinekit/issues/467

---

When executing the M67 command I found that it only works for outputs 0-31. The M68 command however works fine.

[ArcEye]

Comment by luminize Thu Jan 29 14:11:52 2015

---

by the docs info you should be able to have 16 analog inputs, so if you're trying the 32 I/O you’re either lucky to have the M68 working, or the docs are not up to date.

documented here: https://github.com/machinekit/machinekit-docs/blob/master/src/config/emc2hal.asciidoc#options

On 29 Jan 2015, at 14:46, Alexander Rössler notifications@github.com wrote:

When executing the M67 command I found that it only works for outputs 0-31. The M68 command however works fine.

— Reply to this email directly or view it on GitHub.

[ArcEye]

Comment by machinekoder Thu Jan 29 22:35:43 2015

---

Documentation must be wrong: https://github.com/machinekit/machinekit-docs/blob/af2d9e850e7117c2c899456277298143f0686cbc/src/gcode/m-code.asciidoc#m68-analog-output Says that it can be configured by the num_aio parameter. The motion component can have up to 64 I/Os.

But maybe the NML handling is different.

[ArcEye]

Comment by machinekoder Thu Jan 29 23:03:26 2015

---

Interesting: https://github.com/machinekit/machinekit/blob/master/src/emc/motion/emcmotcfg.h#L32

[ArcEye]

Comment by machinekoder Thu Jan 29 23:27:40 2015

---

https://github.com/machinekit/machinekit/blob/master/src/emc/motion/mot_priv.h#L258 https://github.com/machinekit/machinekit/blob/master/src/emc/nml_intf/emcglb.h#L30 Should be 64.

Maybe the data type is not 64bits long on the ARM platform: https://github.com/machinekit/machinekit/blob/master/src/emc/tp/tc_types.h#L78

[ArcEye]

Comment by machinekoder Thu Jan 29 23:31:40 2015

---

Hmm, the ARM platform supports the unsigned long long data type as 64bit unsiged integer. http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/Ciabcebf.html

But maybe the assignment needs to be typecast: https://github.com/machinekit/machinekit/blob/master/src/emc/tp/tp.c#L3169

[ArcEye]

Comment by machinekoder Fri Jan 30 08:30:22 2015

---

A quick test on the BBB yielded the result that indeed the type of the shifted number is the problem.

Also fixed the analog IO number in the documentation https://github.com/machinekit/machinekit-docs/pull/16

[ArcEye]

Comment by machinekoder Thu Feb 5 14:57:36 2015

---

Can someone please merge the PR that fixes this issue

[ArcEye]

Comment by machinekoder Fri Feb 13 16:15:51 2015

---

Unfortunately this problem is only partially fixed. Somehow analog outputs > 31 are not always set correctly. When running the program with analog ouput 41 I get wrong results.

[ArcEye]

Comment by luminize Fri Feb 13 16:26:44 2015

---

Do you have an example of what should happen, and what actually happens?

On 13 Feb 2015, at 17:15, Alexander Rössler notifications@github.com wrote:

Unfortunately this problem is only partially fixed. Somehow analog outputs > 31 are not always set correctly. When running the program with analog ouput 41 I get wrong results.

— Reply to this email directly or view it on GitHub.

[ArcEye]

Comment by machinekoder Tue Feb 24 11:52:26 2015

---

Yes and no.

In particular I can compare printing results. When I use a motion pin higher than 31 the results always look like analog-out was either not set to the right value or set too late.

It also happens with analog out 30 for example, but less frequently.

I hope I can track down the error source.

[ArcEye]

Comment by mhaberler Tue Feb 24 14:05:59 2015

---

gut feeling: a 32/64 bit conversion, or sign extension problem

[ArcEye]

Comment by mhaberler Sat Feb 28 11:48:04 2015

---

really a question for @robEllenberg:

this superkludgy way of doing IO in the tp needs eventual fixing, and @strahlex and me were just going over the tp code; we wonder if the following sounds right:

• the proper way would be to make pinops a TC of proper 'tc_motion_type'
• the tp has an assumption that the tcqItem() function ALWAYS returns a move-type motion (TC_LINEAR, TC_CIRCULAR, TC_RIGIDTAP)

assuming we allow non-move tc_motion_type tagged TC's, (e.g. TC_PINIO) so we have that properly queued instead of piggybacked onto a move, would it be enough to modify tcqItem() as follows:

• tcqItem() gets an new bool param, moves_only
• if true, tcqItem() skips foward/backward according to index over non-move TC's
• if false behaviour is unchanged

the only drawback I would see if sequential tc access is driven by index and there's meaning attached to the index value

I would do this in steps:

• extend the tcqItem() signature as above
• make tpSetAout and tpSetDout queue TC's of TC_PINIO type but ignore those when playing them out
• make that configurable for bug hunt
• only after we know for sure the tcqItem() new indexing behavior works as expected actually move the pin wiggling stuff to TC_PINIO type TC's

in essence if the flag is true tcqItem() implements:

• skip forward/backward index move items, skipping non-move items
• if found, return it, else NULL

if false, this would be as is - current behavior

tpRunCycle would eventually play out all non-move TC's before and including the next move TC

Rob - what's your gut feeling on such a change? Are we going to nuke the tp ;-?

[ArcEye]

Comment by mhaberler Sat Feb 28 16:12:55 2015

---

After 2hrs driving the highway the tcqNext() semantics looks as follows:

before

/*! tcqItem() function
 * \brief gets the n-th TC element in the queue, without removing it
 * Function gets called by tpSetVScale(), tpRunCycle(), tpIsPaused()
 * @param    tcq       pointer to the TC_QUEUE_STRUCT
 * @param    n          index position
 * @return   TC_STRUCT returns the TC elements
 */
TC_STRUCT * tcqItem(TC_QUEUE_STRUCT const * const tcq, int n)

after:

/*! tcqItem() function
 * \brief gets the n-th TC element in the queue, without removing it
 * depending on the moves_only parameter, tcqItem() will skip non-move entries
 * (i.e. everything except TC_LINEAR, TC_CIRCULAR, TC_RIGIDTAP)
 * Function gets called by tpSetVScale(), tpRunCycle(), tpIsPaused()
 * @param    tcq       pointer to the TC_QUEUE_STRUCT
 * @param    index   desired (in) and actual  index  (returned) position after skipping non-move entries
 * @param    moves_only  boolean; if true, skip over non-moves
 * @return   TC_STRUCT returns the TC elements
 */
TC_STRUCT * tcqItem(TC_QUEUE_STRUCT const * const tcq, int *index, bool moves_only)

after which walking the tc queue for moves looks like

int index = 0;
    while ((tc = tcqItem(tcq, &index, true)) != NULL) {
      ...process move...
     index += 1; // NB may not be monotonically increasing
     // as non-move entries are skipped
   }

sounds better?

[ArcEye]

Comment by robEllenberg Sat Feb 28 16:18:33 2015

---

There's a philosophical question I'd like to address here too, but first, I'll focus on the implementation you're proposing. What you're describing sounds workable. The queue index has no special meaning in the current TP queue, other than making it convenient to iterate.

Inserting a blend arc betwen two movement commands may get a bit more complicated, however. Right now, we don't have to do explicit insertion, because we create the blend arc "just in time", and append it to the queue before the new line / arc is appended. If IO commands are added to the queue, then we'll be inserting the blend arc after the IO command. This isn't much different that what we're doing now (where IO switch happens at the end of the blend arc), but it's something to consider.

Stepping back a bit, I wonder if we could generalize this to other events that need to be queued, but aren't explicitly motions. The whole queue buster concept seems to be a solution to the lack of non-motion commands in the motion queue. It seems much more natural to have the motion queue be generalized to a "sequential machine actions" queue, and add spindle commands, dwells, etc. For example, spindle off / on commands could have their own type (TC_SPINDLE), and would be completed only when the spindle was at speed. Dwells could also be motion commands if we can do a simple delay (i.e. wait some number of TP cycles). We could flag some types with a wait-for-condition (spindle, dwell, arbitrary hal pin), and they would interrupt blending (i.e. we check for that and abort creating a blend arc).

I'm sure there are consequences and special cases I'm missing, but this could go a long way towards getting rid of emctask's weird coupling with motion. To me, controlling motion behavior by queue throttling (i.e. task's influence over motion through queue busters) is fraught with unintended consequences, and makes things like single-block stepping a huge headache.

Ironically, with this scheme, synced IO stills seems like a special case, at least if we want it to be perfectly synced with blend arcs. The ideal time to trigger is halfway through the blend arc. One solution is splitting the blend into two segments to insert the IO command in between. This means doing actual queue insertion though. Another option is to simply disallow arc blending with IO commands, and fall back to parabolic blending, though this isn't a terribly future-proof idea.

[ArcEye]

Comment by mhaberler Sat Feb 28 17:31:47 2015

---

blend arcs.. that is a good point. Maybe one option is to make behavior configurable, either globally or on a per-set-pin-operation basis, meaning either before or after blend.

Reading the canon code I noticed that the M6x commands actually have parameters like start and end, maybe these could be used to indicate a user preference (they are carried down to motion but nothing is done with them, and they could well be repurposed, as I dont see a use of per-segment pin settings, do you?):

analog https://github.com/machinekit/machinekit/blob/master/src/emc/task/emccanon.cc#L3168-L3182 https://github.com/machinekit/machinekit/blob/master/src/emc/task/emccanon.cc#L3189-L3203

digital: https://github.com/machinekit/machinekit/blob/master/src/emc/task/emccanon.cc#L3121-L3136 https://github.com/machinekit/machinekit/blob/master/src/emc/task/emccanon.cc#L3147-L3161

I absolutely agree that non-movement commands - like your state tags - should be first-class travellers on the motion queue. The whole syncing and line-numbering business is seriously botched - like staring at a not-particularly-descriptive motion ID and concluding 'this seems complete', and is a pretty large sledgehammer to achieve the goal

for that matter, a 'sync' command could just be a tc element as well - and once it's dequeued, using layer is told 'btw sync #suchandsuch just completed', or to cause a status snapshot to be sent upwards of relevant. Sync, IO, state tags, tagging a source line for stepping - it's all the same, they should be handled queued, and ignored when doing trajectory planning.

the nice thing with the loadable tp we now have is: one can fork emc/tp into emc/tp-experimental, built 'tpx', ask folks to run it and have an eye on it, and if there is something fishy, revert to 'loadrt tp' and they are back in stable land

I'll do this (try the new tcqItem indexing, (maybe optionally) queue a TC_PINIO on pin I/O) once I see the light with the HAL instance stuff, which is coming along faster than I thought (yeah, light at end of tunnel is from train in opposite direction ;)

once we're above water with the new indexing behavior, all sorts of stuff can be tacked on here