Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log facilities #690

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,16 @@ The *modbus_strerror()* function is provided to translate libmodbus-specific
error codes into error message strings; for details refer to
[modbus_strerror](modbus_strerror.md).

## Extended debug

By default, the debug output created by enabling [modbus_set_debug](modbus_set_debug.md)
is written to stdout/stderr. With the following functions those can be redirected
to files or a callback.

- [modbus_set_out_user_data](modbus_set_out_user_data.md)
- [modbus_set_error_user_data](modbus_set_error_user_data.md)
- [modbus_set_trace_handler](modbus_set_trace_handler.md)

## Miscellaneous

To deviate from the Modbus standard, you can enable or disable quirks with:
Expand Down
32 changes: 32 additions & 0 deletions docs/modbus_set_error_user_data.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# modbus_set_error_user_data

## Name

modbus_set_error_user_data - set file stream to write log output

## Synopsis

```c
void modbus_set_error_user_data(modbus_t *ctx, void* out_user_data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these really void* or should they explicitly be FILE * pointers? The names and the examples and the descriptions don't really line up well here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is form the original PR. This also was the "usual" way to pass arbitrary data for callback when I last did "C", so I didn't change it.

```

## Description

The *modbus_set_error_user_data()* changes where log output is written
to when enabled with [modbus_set_debug](modbus_set_debug.md). Defaults
to stderr when not set.


## Example

```c
FILE *fp;
fp = fopen("error.txt", "w");
modbus_set_error_user_data(ctx, fp);
modbus_set_debug(ctx, 1)
```

## See also

- [modbus_set_out_user_data](modbus_set_out_user_data.md)
- [modbus_set_trace_handler](modbus_set_trace_handler.md)
32 changes: 32 additions & 0 deletions docs/modbus_set_out_user_data.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# modbus_set_out_user_data

## Name

modbus_set_out_user_data - set file stream to write log output

## Synopsis

```c
void modbus_set_out_user_data(modbus_t *ctx, void* out_user_data);
```

## Description

The *modbus_set_out_user_data()* changes where log output is written
to when enabled with [modbus_set_debug](modbus_set_debug.md). Defaults
to stdout when not set.


## Example

```c
FILE *fp;
fp = fopen("output.txt", "w");
modbus_set_out_user_data(ctx, fp);
modbus_set_debug(ctx, 1)
```

## See also

- [modbus_set_error_user_data](modbus_set_error_user_data.md)
- [modbus_set_trace_handler](modbus_set_trace_handler.md)
44 changes: 44 additions & 0 deletions docs/modbus_set_trace_handler.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# modbus_set_trace_handler

## Name

modbus_set_trace_handler - call method when log data is written

## Synopsis

```c
typedef int (*modbus_stream_handler_t)(void *user, const char *format, va_list ap);
Copy link
Contributor

@karlp karlp Mar 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to document the return type, at least. Again, void *user vs FILE * is... muddy here. I know you want void * user for when you have a custom handler, and FILE* magically working when you just set the "user data" without registering a custom handler, but this is... very unclear to people not well versed in these things.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uses printf return type / values. I don't know how to document the void pointer stuff properly, this is a concept you have to understand in order to not make errors...

void modbus_set_trace_handler(modbus_t *ctx, modbus_stream_handler_t handler);
```

## Description

The *modbus_set_trace_handler()* sets a callback. When log data is written, the
callback is called. A log message is finalized with a '\n' as last character.
Defaults to vfprintf when not set.


## Example

```c++
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a C library, I think a plain boring C example is more suitable. C++ as well, sure, ifyou think it's important, but I don't think the only example should be c++

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's copy/paste from my application code. Don't have a C example at hand.

class Test {
public:
static int log_callback(void *user, const char *format, va_list ap)
{
auto *inst = reinterpret_cast<Test*>(user);
//call methods
}
void setup()
{
modbus_set_out_user_data(reinterpret_cast<void*>(this));
modbus_set_error_user_data(reinterpret_cast<void*>(this));
modbus_set_trace_handler(log_callback);
modbus_set_debug(true);
}
}
```

## See also

- [modbus_set_out_user_data](modbus_set_out_user_data.md)
- [modbus_set_error_user_data](modbus_set_error_user_data.md)
4 changes: 3 additions & 1 deletion src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ libmodbus_la_SOURCES = \
modbus.h \
modbus-data.c \
modbus-private.h \
modbus-log.c \
modbus-log.h \
modbus-rtu.c \
modbus-rtu.h \
modbus-rtu-private.h \
Expand All @@ -35,7 +37,7 @@ endif

# Header files to install
libmodbusincludedir = $(includedir)/modbus
libmodbusinclude_HEADERS = modbus.h modbus-version.h modbus-rtu.h modbus-tcp.h
libmodbusinclude_HEADERS = modbus.h modbus-version.h modbus-log.h modbus-rtu.h modbus-tcp.h

DISTCLEANFILES = modbus-version.h
EXTRA_DIST += modbus-version.h.in
Expand Down
82 changes: 82 additions & 0 deletions src/modbus-log.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
#include "modbus-log.h"
#include "modbus-private.h"
#include "modbus.h"

#include <errno.h>
#include <stdio.h>
#include <stddef.h>

MODBUS_API void modbus_set_out_user_data(modbus_t *ctx, void* out_user_data)
{
if (ctx == NULL) {
errno = EINVAL;
return;
}
ctx->out_user_data = out_user_data;
}

MODBUS_API void modbus_set_error_user_data(modbus_t *ctx, void* error_user_data)
{
if (ctx == NULL) {
errno = EINVAL;
return;
}
ctx->error_user_data = error_user_data;
}

MODBUS_API void modbus_set_trace_handler(modbus_t *ctx, modbus_stream_handler_t handler)
{
if (ctx == NULL) {
errno = EINVAL;
return;
}
ctx->stream_handler = handler;
}

MODBUS_API int modbus_trace(modbus_t *ctx, const char* format, ...)
{
int result;
va_list argp;
va_start(argp, format);

result = modbus_vtrace(ctx, format, argp);

va_end(argp);
return result;
}

MODBUS_API int modbus_vtrace(modbus_t *ctx, const char* format, va_list ap)
{
if (ctx == NULL || ctx->stream_handler == NULL) {
errno = EINVAL;
return -1;
}
if (!ctx->out_user_data) {
ctx->out_user_data = stdout;
}
return ctx->stream_handler(ctx->out_user_data, format, ap);
}

MODBUS_API int modbus_trace_error(modbus_t *ctx, const char* format, ...)
{
int result;
va_list argp;
va_start(argp, format);

result = modbus_vtrace_error(ctx, format, argp);

va_end(argp);
return result;
}

MODBUS_API int modbus_vtrace_error(modbus_t *ctx, const char* format, va_list ap)
{
if (ctx == NULL || ctx->stream_handler == NULL) {
errno = EINVAL;
return -1;
}
if (!ctx->error_user_data) {
ctx->error_user_data = stderr;
}
return ctx->stream_handler(ctx->error_user_data, format, ap);
}
19 changes: 19 additions & 0 deletions src/modbus-log.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#ifndef MODBUS_LOG_H
#define MODBUS_LOG_H

#include "modbus.h"

#include <stdarg.h>

MODBUS_API void modbus_set_out_user_data(modbus_t *ctx, void* out_user_data);
MODBUS_API void modbus_set_error_user_data(modbus_t *ctx, void* error_user_data);

typedef int (*modbus_stream_handler_t)(void *user, const char *format, va_list ap);
MODBUS_API void modbus_set_trace_handler(modbus_t *ctx, modbus_stream_handler_t handler);

MODBUS_API int modbus_trace(modbus_t *ctx, const char* format, ...);
MODBUS_API int modbus_vtrace(modbus_t *ctx, const char* format, va_list ap);
MODBUS_API int modbus_trace_error(modbus_t *ctx, const char* format, ...);
MODBUS_API int modbus_vtrace_error(modbus_t *ctx, const char* format, va_list ap);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these last 4 really part of the api?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those are form the original PR. I have no idea where else to put them / how to make them private. There is no harm if you call them.


#endif /* MODBUS_LOG_H */
4 changes: 4 additions & 0 deletions src/modbus-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ struct _modbus {
struct timeval indication_timeout;
const modbus_backend_t *backend;
void *backend_data;
/* redirect for logging */
void* out_user_data;
void* error_user_data;
modbus_stream_handler_t stream_handler;
};

void _modbus_init_common(modbus_t *ctx);
Expand Down
Loading