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),