Added Marlin ADVANCED_OK + static ADVANCED_OK support

[digant73]

IMPROVEMENTS:

• Added advanced_ok setting on config.ini file to allow Marlin ADVANCED_OK and static (TFT based) ADVANCED_OK support: Based on original code shared by @rondlh on [FR] ADVANCED_OK support, base code implemented already #2819. Code reviewed, fixed and improved

  If enabled:

  • if "ADVANCED_OK" feature is enabled in Configuration_adv.h in Marlin firmware, the TFT will use
    the available G-code TX slots indication provided by the mainboard to schedule the transmission
    of multiple G-codes, if any, for a maximum of the given indication.
  • if "ADVANCED_OK" feature is disabled in Configuration_adv.h in Marlin firmware, the TFT will
    support the transmission of G-codes according to the configured tx_slots setting.

  If disabled, the TFT will provide the standard transmission logic based on one G-code per time.

• Added tx_slots setting on config.ini file: Maximum number of G-code TX slots used by the TFT for the communication with the printer used when static ADVANCED_OK is applied

• Added Monitoring menu: Available in Notification menu (pressing on Info button) only if DEBUG_MONITORING option is enabled in Configuration.h. It allows to monitor/show resources usage (e.g. buffered gcodes, pending gcodes, available TX slots etc...) useful for troubleshooting (see attached pictures)

• Minor code optimization on os_timer.c: As reported in FR [BUG] Buggy time handling code (doesn't consider timer rollover) #2832, useless invokation of updatePrintTime() function was performed while it could be limited just to 1 per second (as needed during a printing)

• Minor improvement in _GUI_DispLenString() function (GUI API) when displaying a truncated message

• Minor code cleanup/optimization

NOTES:

• Backward compatibility with current gcode transmission logic based on one command per time is supported/allowed simply by setting advanced_ok to 0 (disabled)
• ADVANCED_OK feature made configurable also at runtime on Feature menu
• Good print quality results for static ADVANCED_OK seem to be reached even with tx_slots set to 2 (no frequent idle state on mainboard's RX buffer

BUGFIXES:

• Bugfix on print abort loop: Function Serial_Available provided by last merged PR Fixed random pause/hesitation when printing #2827 was not enough to always properly handle a print abort loop. It was possible to flood M108 causing buffer overruns on Marlin side and related errors displayed on TFT as popup error messages
• No TFT connection established at TFT boot when a SMART runout sensor is enabled: With a SMART runout sensor enabled, the TFT was not able to connect at boot (it was necessary to use the disconnect button on Connection menu to properly connect the TFT to mainboard). The TFT sent a M114 E gcode for which mainboard didn't provide any reply. Bug is more on mainboard than on TFT but it is fixed on TFT side simply delaying the transmission of M114 E after the TFT properly connected to mainboard

resolves #2819
resolves #2806 (zombie)
resolves #2810 (zombie)
resolves #2816 (zombie)

PR STATE: Ready for merge

[rondlh]

Here a small tweak for the notification list (Gui.c)
If you like it please feel free to add it to any PR you like.

It checks if the truncate indicator "…" is actually needed, it's not needed if:

1. the last character is already displayed.
2. the only missing character fits into the available space reserved for the truncate indicator.

Let me show what it does by giving some examples.
"Date:2023-09-18 18:21:34" (The complete notification message fits into the available space)
"Date:2023-09-18 18:21:34…" (OLD Listview menu displays this, message is complete but truncate indicator is added)
"Date:2023-09-18 18:21:34 " (NEW, message is complete, no need for truncate indicator)

"Date:2023-09-18 18:21:34A" (The complete notification message just fits into the available space)
"Date:2023-09-18 18:21:34…" (OLD Listview menu displays this, truncates message)
"Date:2023-09-18 18:21:34A" (NEW, use the truncate space for the last character)

"Date:2023-09-18 18:21:34AB" (The complete notification message doesn't fit)
"Date:2023-09-18 18:21:34…" (OLD Listview menu displays this, CORRECT)
"Date:2023-09-18 18:21:34…" (No change)

The 2 changed lines have comments.

const uint8_t* _GUI_DispLenString(int16_t x, int16_t y, const uint8_t *p, uint16_t pixelWidth, bool truncate)
{
  if (p == NULL) return NULL;

  CHAR_INFO info;
  uint16_t curPixelWidth = 0;

  if (truncate) pixelWidth -= BYTE_HEIGHT;

  while (curPixelWidth < pixelWidth && *p)
  {
    getCharacterInfo(p, &info);
    if (curPixelWidth + info.pixelWidth > pixelWidth)
    {
      if ((truncate) && *(p + 1)) // check if there is at least 1 more character
      {
        if (*(p + 2) || info.pixelWidth > BYTE_HEIGHT) // truncate indicator if 2 more characters OR not enough space for current character in reserved space
          getCharacterInfo((uint8_t *)"…", &info);
        GUI_DispOne(x, y, &info);
      }

      return p;
    }
    GUI_DispOne(x, y, &info);
    x += info.pixelWidth;
    curPixelWidth += info.pixelWidth;
    p += info.bytes;
  }
  return p;
}

[rondlh]

I noticed that you made the whole DMA_CIRCULAR_BUFFER volatile, but I believe that it's only required for the members that are actually changed in the ISR, so wIndex only.
It's not efficient for cacheSize and cache for example, they could even be defined constant to help the compiler.
Any reason to do this?

typedef volatile struct  // precautionally declared as volatile due to access from interrupt handler and main thread
{
  char *cache;
  uint16_t wIndex;  // writing index
  uint16_t rIndex;  // reading index
  uint16_t flag;    // custom flag (for custom usage by the application)
  uint16_t cacheSize;
} DMA_CIRCULAR_BUFFER;

[digant73]

I noticed that you made the whole DMA_CIRCULAR_BUFFER volatile, but I believe that it's only required for the members that are actually changed in the ISR, so wIndex only. It's not efficient for cacheSize and cache for example, they could even be defined constant to help the compiler. Any reason to do this?

typedef volatile struct  // precautionally declared as volatile due to access from interrupt handler and main thread
{
  char *cache;
  uint16_t wIndex;  // writing index
  uint16_t rIndex;  // reading index
  uint16_t flag;    // custom flag (for custom usage by the application)
  uint16_t cacheSize;
} DMA_CIRCULAR_BUFFER;

yes, volatile could be applied only to wIndex. I would provide this change in the PR related to #2835. const cannot be applied to cache or cacheSize (warning and compile error)

[digant73]

if (*(p + 2) || info.pixelWidth > BYTE_HEIGHT) // truncate indicator if 2 more characters OR not enough space for current character in reserved space
getCharacterInfo((uint8_t *)"…", &info);
GUI_DispOne(x, y, &info);

shouldn't it be:

if (*(p + 2) || info.pixelWidth > BYTE_HEIGHT) // truncate indicator if 2 more characters OR not enough space for current character in reserved space
{
  getCharacterInfo((uint8_t *)"…", &info);
  GUI_DispOne(x, y, &info);
}

?

EDIT: forget it

Ok, minor improvement added

[rondlh]

Ok, minor improvement added

THANKS! I know it's a minor thing, but my standard startup messages (date and time, ip address, ETA etc) have this issue, so it annoyed me a bit.

[rondlh]

@digant73
I found a potential issue... it could be me, but it seems that the BTT FW only sends "M105\n" once, and when it doesn't get a response then you are out of luck. (I can see exactly what arrives at the host)
I have the latest Marlin (about 3 weeks old) + #2824 up-to-date.

Perhaps it's the Marlin bug you mentioned?
I simulate this by resetting the motherboard, and then 2 seconds later I reset the TFT.
Buffered gcodes and pending gcodes = 1. tx_slots = 0.
The TFT Disconnect solves the issue like you mentioned, because another "M105" is sent.
When I reset the TFT again, then they connect fine, which makes me think that it's not an Marlin issue, but a TFT issue.
Could some condition be blocking the sending of the repeated "M105"?

UPDATE: When going from the current master to #2824, then I always need to use "reset.txt" to get things going.
So I update all icons/fonts/ini etc, then get a black screen and get stuck. Using reset.txt gets things going again.

UPDATE 2: It might be wise to add some keywords to the language now file for future use.
"Gcode Checksum"
"Compact Gcode", a feature to remove spaces from the Gcode commands.

[digant73]

@digant73 I found a potential issue... it could be me, but it seems that the BTT FW only sends "M105\n" once, and when it doesn't get a response then you are out of luck. (I can see exactly what arrives at the host) I have the latest Marlin (about 3 weeks old) + #2824 up-to-date.

Perhaps it's the Marlin bug you mentioned? I simulate this by resetting the motherboard, and then 2 seconds later I reset the TFT. Buffered gcodes and pending gcodes = 1. tx_slots = 0. The TFT Disconnect solves the issue like you mentioned, because another "M105" is sent. When I reset the TFT again, then they connect fine, which makes me think that it's not an Marlin issue, but a TFT issue.

UPDATE: When going from the current master to #2824, then I always need to use "reset.txt" to get things going. So I update all icons/fonts/ini etc, then get a black screen and get stuck. Using reset.txt gets things going again.

strange. Try to disable ADVANCED_OK in Marlin and verify if the issue is still present

EDIT: bug fixed

[rondlh]

Confirmed, fixed, if not connected then M105 keep coming, well done!
I guess the issues with the SMART runout sensor were also related to this issue:
"Fixed no TFT connection issue with SMART runout sensor enabled".

Let me check if my ESP3D issue (early ESP3D message stops the connection) is fixed now too
Confirmed, that works fine!