linuxcnc.stat 'mcodes' attribute is broken

[Sigma1912]

Tested on RIP installs of current 2.9 and master and a package install of 2.9.3.

Here are the steps I follow to reproduce the issue:

1. Start sim config eg 'axis' and home the machine
2. Open 'LinuxCNC Status' window and scroll to 'mcodes' attribute OR whatch the 'Active Gcodes panel in the AxisGui'
3. Start the preloaded Gcode

This is what I expected to happen:

I would not expect 'M0' to be shown as active
I would expect 'M5' to change to 'M3' as the gcode executes

This is what happened instead:

'M0' is shown as active
'M5' quickly changes to 'M4' (!) and then back to 'M5' as the gcode executes

It worked properly before this:

Not sure if this worked before current version 2.9

[Sigma1912]

Testing MDI commands

• M3 S1000
• M0
  Shows 'M3'
  Shows 'M0' twice
  

Which makes me think that the 'M0' at the end (for one thing) should really not be there at all.

[Sigma1912]

stat.mcodes array for the above situation is: (0, -1, 3, -1, 9, -1, 48, -1, 53, 0)

Looking at this code I'm having trouble figuring out where the '0' at the end of the above array comes from:

[Sigma1912]

Looking at this makes me think the last entry in the mcodes array might be for group 10 'user defined':

However there is apparently nothing in the source code like 'active_m_codes[9] = ' that would set that.

[rmu75]

I guess that m_codes array (size 10) is zero-initialized, and nothing is writing to array index 9, so therefore that always contains 0 and will be displayed as M0. should go away with array initialized to -1. why it was designed that way... who knows. would be cleaner to replace that (and active g-codes) with an actual std::vector but that probably won't fly with NML.

[Sigma1912]

Thanks for your comment. Problem is I cannot seem to find the initialization of the array.

My current 'solution' is to simply not copy the last element:

diff --git a/src/emc/rs274ngc/rs274ngc_pre.cc b/src/emc/rs274ngc/rs274ngc_pre.cc
index 6616a1c7c2..d2aa3c1c61 100644
--- a/src/emc/rs274ngc/rs274ngc_pre.cc
+++ b/src/emc/rs274ngc/rs274ngc_pre.cc
@@ -2134,7 +2134,10 @@ void Interp::active_m_codes(int *codes)        //!< array of codes to copy into
 {
   int n;
 
-  for (n = 0; n < ACTIVE_M_CODES; n++) {
+  // The last entry in the array seems be always zero which shows in the stat.mcodes attributes
+  //  and falsely indicates an active 'M0' so we don't copy that last element
+  for (n = 0; n < ACTIVE_M_CODES-1; n++) {
     codes[n] = _setup.active_m_codes[n];
   }
 }

[Sigma1912]

Testing the spindle mcodes with this gcode on RIP master and deb install 2.9.3:

m5
g0 x0
m0
m2

gives me this (while it waits at 'm0'), Note the 'm0' at the end is because of the bug discussed above:

same for (while waiting at 'g4 p5' )

m5
g0 x0
g4 p5
m2

[rmu75]

Thanks for your comment. Problem is I cannot seem to find the initialization of the array.

In C++ that happens implicitly (to 0)

[Sigma1912]

Just be sure this comes from here, I added a debug printf:

And it seems that somehow the spindle flag is on:

GM_FLAG_SPINDLE_ON 1 

In C++ that happens implicitly (to 0)

Ah I see.

[Sigma1912]

This looks like a contradiction to me:

In interp_write.cc: 314

    state.flags[GM_FLAG_SPINDLE_ON] =
	!(settings->spindle_turning[0] != CANON_STOPPED);

rs274ngc_pre.cc: 2214

    m_codes[2] = !tag.flags[GM_FLAG_SPINDLE_ON] ? 5 :

If I combine the two sections this would be equivalent to:

 m_codes[2]  = ! (!(settings->spindle_turning[0] != CANON_STOPPED) ? 5) :

Dropping the double negation:

m_codes[2] = (settings->spindle_turning[0] != CANON_STOPPED) ? 5 :

But in rs274ngc_pre.cc: 163 (which looks correct)

  settings->active_m_codes[2] = (settings->spindle_turning[0] == CANON_STOPPED) ? 5 :   

[Sigma1912]

This seems to indeed fix the spindle field:

diff --git a/src/emc/rs274ngc/interp_write.cc b/src/emc/rs274ngc/interp_write.cc
index 82010475d7..cd5d0b10d6 100644
--- a/src/emc/rs274ngc/interp_write.cc
+++ b/src/emc/rs274ngc/interp_write.cc
@@ -312,7 +312,7 @@ int Interp::write_state_tag(block_pointer block,
        (block == NULL) ? -1 : block->m_modes[4];
 
     state.flags[GM_FLAG_SPINDLE_ON] =
-       !(settings->spindle_turning[0] != CANON_STOPPED);
+       (settings->spindle_turning[0] != CANON_STOPPED);
     state.flags[GM_FLAG_SPINDLE_CW] =
        (settings->spindle_turning[0] == CANON_CLOCKWISE);

As for the erroneous '0' in the last array element:

I guess that m_codes array (size 10) is zero-initialized, and nothing is writing to array index 9, so therefore that always contains 0 and will be displayed as M0. should go away with array initialized to -1.

This seems to fix the issue (not sure if there is a more elegant solution though) :

diff --git a/src/emc/rs274ngc/interp_setup.cc b/src/emc/rs274ngc/interp_setup.cc
index 5131d6ce03..852510ed51 100644
--- a/src/emc/rs274ngc/interp_setup.cc
+++ b/src/emc/rs274ngc/interp_setup.cc
@@ -49,7 +49,7 @@ setup::setup() :
     w_origin_offset(0.0),
 
     active_g_codes{},
-    active_m_codes{},
+    active_m_codes{-1,-1,-1,-1,-1,-1,-1,-1,-1,-1},
     active_settings{},
 
     arc_not_allowed(0),

[Sigma1912]

I've been looking into getting 'M0' and 'M1' tag to show when running Gcode. The problem seems to be that while Gcode execution is paused due to 'm0' or 'm1' the status information given is from the line before the actual 'M0'/'M1' command.

[andypugh]

I think that StateFlags was imported from a version of LinuxCNC without multiple spindles (ie, Pathpilot)
So currently SPINDLE_ON only looks at spindle[0], which isn't ideal.
But it seems that this is (for a change) not related to the multispindle patches.

[rmu75]

would it make sense to set SPINDLE_ON to 1 if at least one spindle is running? (OR of all spindle-states). not so clear what to do with CW/CCW.

[rmu75]

I've been looking into getting 'M0' and 'M1' tag to show when running Gcode. The problem seems to be that while Gcode execution is paused due to 'm0' or 'm1' the status information given is from the line before the actual 'M0'/'M1' command.

M0/M1 command doesn't get a state tag so at the realtime end nobody knows about it. compare https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/task/taskintf.cc#L1397 and https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/task/taskintf.cc#L1304. Though I'm not sure if simply setting the state tag in emcTrajPause will be enough.

[andypugh]

would it make sense to set SPINDLE_ON to 1 if at least one spindle is running? (OR of all spindle-states)

This is what happens in a number of other places where the data structure dues not allow for individual spindle statuses.

[Sigma1912]

M0/M1 command doesn't get a state tag so at the realtime end nobody knows about it.

Seems only commands with actual machine movment get a state tag:

• emcTrajLinearMove(
• emcTrajCircularMove(
• emcTrajProbe(
• emcTrajRigidTap(

which is probably why the 'current_line' attribute seems to always point to the line with the last actual move.

[Sigma1912]

Although 'localEmcTrajTag' seems to only get updated in 'emcTrajLinearMove(' and 'emcTrajCircularMove('.

[Sigma1912]

I've added two debug prinf statements like this:

I'm running this Gcode:

m3 s100
m7
g0 x1 
m1
g0 x0
m9
m0
m5
m2

And it looks like the 'EMC_TRAJ_PAUSE_TYPE' case in 'emctaskmain.cc' is not executed at all:

note: jog_invert=set()
task: main loop took 0.021354 seconds
task: main loop took 0.021812 seconds
 case EMC_TRAJ_LINEAR_MOVE_TYPE
 case EMC_TRAJ_LINEAR_MOVE_TYPE

[rmu75]

an additional problem is that the interpreter is coalescing commands that have no side-effects, that's the reason you can only single-step (if it works at all) from one move to the next.

[rmu75]

you can turn on debugging of "interpreter" and "interpreter list", that shows all communication from interp. The message you are looking for is EMC_TASK_PLAN_OPTIONAL_STOP or EMC_TASK_PLAN_PAUSE and those don't come with a state tag.

[rmu75]

Also look at master, I refactored a bunch of that stuff to prepare for an alternative transport besides NML, and things do look a bit different there (using std::unique_ptr etc... instead of naked pointers with non-obvious ownership / lifetime)

[Sigma1912]

Thanks , I am using master.