Provide `Print` implementation instead of `debug_message`

[tig]

I make extensive use of ArduinoLog (https://github.com/thijse/Arduino-Log/).

It can log to any Print implemenation.

It'd be great if avr_debug included a Print implementation that called debug_message.

Thanks.

[tig]

Here's a dorky POC:

This could be added to avr8-stub.c, but I just did it in my main.cpp:

#define DEBUG_PRINT_BUFFER_SIZE 254
/* Convert number 0-15 to hex */
#define nib2hex(i) (uint8_t)((i) > 9 ? 'a' - 10 + (i) : '0' + (i))

class DebugPrint : public Print {
private:
  uint8_t buff[DEBUG_PRINT_BUFFER_SIZE + 1];
  uint8_t len = 0;

public:
  size_t write(uint8_t c) {
    write(&c, 1);
  }

  size_t write(const uint8_t *buffer, size_t size) {
    size_t written = 0;

   // Put a '0' at front to make it a valid GDB debug message
    if (len == 0) {
      buff[len++] = 'O'; /* message to display in gdb console */
    }

    //fill buffer until `\n` found and then send
    for (uint8_t i = 0; i < size; i++) {
      if (buffer[i] == '\n') {
        // Send what we got so far
        // Our buffer, with the message formatted as hex
        // Example packet we send:
        // $O48656c6c6f2c20776f726c64210a#55 = Hello, world!\n
        debug_message_gdb(buff, len);

        // reset buffer
        len = 1;
      } else {
        char c = buffer[i];
        buff[len++] = nib2hex((c >> 4) & 0xf);
        buff[len++] = nib2hex((c >> 0) & 0xf);

        if (len >= DEBUG_PRINT_BUFFER_SIZE) {
          debug_message("exceeded DEBUG_PRINT_BUFFER_SIZE");
          // too bad
          len = 1;
          break;
        }
      }
    }

    return written;
  };
};

I added this function to avr8.stub.c.

/*
 * Send text message, formatted as a gdb debug message (0HHHHHH), to gdb console.
 * Note: GDB will queue the messages until "\n" (0x0a) is received; then
 * it displays the messages in console.
 * 
 */
void debug_message_gdb(const uint8_t* buff, uint8_t size) {
    uint8_t cSREG;
    cSREG = SREG; /* store SREG value */
    cli();  /* disable interrupts */

    uint8_t sum = 0;

    putDebugChar('$');
    while ( size-- > 0)
    {
        putDebugChar(*buff);
        sum += *buff;
        buff++;
    }

    putDebugChar('#');
    putDebugChar(nib2hex((sum >> 4) & 0xf));
    putDebugChar(nib2hex(sum & 0xf));

    SREG = cSREG; /* restore SREG value (I-bit) */
}

Example output from calls to Log.trace and Log.traceln:

PlatformIO: Initialization completed
TV Slider - By Erik Olsen
012345678_012345678_012345678_012345678_
012345678_012345678_012345678_012345678_012345678_
012345678_012345678_012345678_012345678_012345678_012345678_
012345678_012345678_012345678_012345678_012345678_012345678_012345678_
012345678_012345678_012345678_012345678_012345678_012345678_012345678_012345678_
Api::begin
Setting up Ethernet and Server
  Failed to configure Ethernet using DHCP
  Will use fixed IP address instead
  IP Address : 0.0.0.0
REST Functions :  deploy  retract  status  open  close  reset  abort 
Setting up Keypad
Setting up linear actuator motor control relays
  LA motor control relay 1 ready
  LA motor control relay 2 ready
Setup complete : awaiting your command
Setting up state machine
State Machine transitions added - free memory: 1008
Enabling LED
setup done
startup(): Deployed | Cmd: None | Door: Closed | Slider: Deployed, Fwd: 130mm, Rwd: 1500mm, Speed: 0, Sp Fract: 0
exiting startup()
exceeded DEBUG_PRINT_BUFFER_SIZE
on_exit: state_startup
on_enter: state_deployed
>

The major limitation fo all this is it appears GDB can only receive 48 chars at a time.

[jdolinay]

Thanks. Implementing Print is interesting idea. But I think it would be better to provide it separately rather than in the avr8-stub.c because it would require using C++ for the stub. The stub itself is now implemented in C so it can be used in C programs, no C++ required. It would also increase the size of the stub with code that many users may never need.

I'd implement it in a header file that the users could add to their project if they wanted to have the Print implemented. This file could look like this:

#include "Print.h"
#include <inttypes.h>
#include "avr8-stub.h"

class DebugPrint : public Print {
public:
    virtual size_t write(uint8_t c) {
        return write((const char *)&c, 1);
    }

    virtual size_t write(const char *str) {
        debug_message(str);
        return strlen(str);
    }

    size_t write(const char *buffer, size_t size) {
        // debug_message expects null terminated string, which buffer may
        // not be, so make a copy with terminator
        char *p = new char[size+1];
        memcpy(p, buffer, size);
        p[size] = '\0';
        debug_message(p);
        delete p;
        return size;
    }
};

Then in your main.cpp file you would have:

#include "DebugPrint.h"
DebugPrint print;

and in setup():

Log.begin (LOG_LEVEL_VERBOSE, &print);

I never used the Arduino-Log so I may be missing something. What do you think?

[tig]

Generally, works for me.

However, isn't allocating memory dynamically bad ju-ju for embedded programming?

Shall I do a PR?

[tig]

Oh, and debug_message() won't work for this as-is. It his hard coded to not send more than (AVR8_MAX_BUFF-4)/2 chars, which is 47 on avr.

See #38.

/*
 * Send text message to gdb console.
 * Note: GDB will queue the messages until "\n" (0x0a) is received; then
 * it displays the messages in console.
 */
void debug_message(const char* msg)
{
    uint8_t cSREG;
    char c;
    uint8_t i = 0;
    cSREG = SREG; /* store SREG value */
    cli();  /* disable interrupts */

    gdb_ctx->buff[i++] = 'O';   /* message to display in gdb console */
    while ( *msg && i < (AVR8_MAX_BUFF-4) )
    {
        c = *msg++;
        gdb_ctx->buff[i++] = nib2hex((c >> 4) & 0xf);
        gdb_ctx->buff[i++] = nib2hex((c >> 0) & 0xf);
    }

    /* Add "\n" */
    gdb_ctx->buff[i++] = '0';
    gdb_ctx->buff[i++] = 'a';

    gdb_send_buff(gdb_ctx->buff , i);

    SREG = cSREG; /* restore SREG value (I-bit) */
    /* Example packet we send:
     $O48656c6c6f2c20776f726c64210a#55 = Hello, world!\n
     */
}

Note also that my POC above addresses another requirement I didn't make clear up front:

It is common for libraries like AdruinoLog to support print() and println() like functions where only printlin() adds the terminaing \n. Thus, it's common for folk to do stuff like this:

    Log.notice(F("I2C device found at addresses: "));
    for (address = 1; address < 127; address++) {
...
      if (error == 0) {
        Log.notice(F("%X, "), address);

Which will output "I2C device found at addresses: 0x34, 0x55, " on ONE LINE.

My POC above addresses this by using a static buffer and only calling into avr_debug to send the string when a \n is encoutered.

[jdolinay]

However, isn't allocating memory dynamically bad ju-ju for embedded programming?

Yes, I don't like it either. Just could not think of fast way to solve the problem that there is buffer needed with +1 char to terminate the string. Your solution with buffer in the class will be better. It will take up extra RAM but the user will know that using the DebugPrint class means having more RAM used.

Shall I do a PR?

If you could do it the separate file with the DebugPrint class that would be great. I guess it can be all in header without .cpp file to make it easier to use. I would probably put the file in some new folder rather than together with the other sources so that it is not included automatically into each project, but that can be decided later.

I will look at the limit of chars in debug_message and reply in the issue #38.

[jdolinay]

Sorry, I did not understand your code completely until now. Now I finally understand. And I think the best way to implement this would be:

• use your implementation of the DebugPrint class from the first comment above.
• add your function debug_message_gdb to the avr8-stub.c, so that your DebugPrint class could work.
• OR add the debug_message_gdb as a private function to the DebugPrint class? Is there a reason it should be in the avr8-stub.c? If yes, no problem - it could be inside conditional compilation block, enabled only if the DebugPrint header is included, so it would have no effect on the size of the existing code.

Sorry about the confusion, I should have looked at your code more closely at the beginning. Please prepare pull request when you have a chance.

As to the issue #38, I think with the above applied it will have no effect on your code., you will not need to use the debug_message, right? In this case I would leave the debug_message as it is. To avoid risk of breaking what already works and extra work :)

[mhmast]

any news on this?

[jdolinay]

Hi, I was waiting for pull request from @tig. I think this would be nice-to-have feature but not many users really need it, so I did not get to work on it myself.. Would this be useful for you? Perhaps you can use the code from the original tig's post above (which I guess is working); it is just not built into the library.