Skip to content

Cellular: Improve ATHandler AT debug traces #10121

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 3, 2019
Merged

Cellular: Improve ATHandler AT debug traces #10121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
54 changes: 33 additions & 21 deletions features/cellular/framework/AT/ATHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ bool ATHandler::fill_buffer(bool wait_for_timeout)
// Reset buffer when full
if (sizeof(_recv_buff) == _recv_len) {
tr_error("AT overflow");
debug_print(_recv_buff, _recv_len);
debug_print(_recv_buff, _recv_len, AT_ERR);
reset_buffer();
}

Expand All @@ -379,7 +379,7 @@ bool ATHandler::fill_buffer(bool wait_for_timeout)
if (count > 0 && (fhs.revents & POLLIN)) {
ssize_t len = _fileHandle->read(_recv_buff + _recv_len, sizeof(_recv_buff) - _recv_len);
if (len > 0) {
debug_print(_recv_buff + _recv_len, len);
debug_print(_recv_buff + _recv_len, len, AT_RX);
_recv_len += len;
return true;
}
Expand Down Expand Up @@ -481,7 +481,6 @@ ssize_t ATHandler::read_bytes(uint8_t *buf, size_t len)
}
buf[read_len] = c;
if (_debug_on && read_len >= DEBUG_MAXLEN) {
debug_print(DEBUG_END_MARK, sizeof(DEBUG_END_MARK) - 1);
_debug_on = false;
}
}
Expand Down Expand Up @@ -586,7 +585,6 @@ ssize_t ATHandler::read_hex_string(char *buf, size_t size)
int c = get_char();

if (_debug_on && read_idx >= DEBUG_MAXLEN) {
debug_print(DEBUG_END_MARK, sizeof(DEBUG_END_MARK) - 1);
_debug_on = false;
}

Expand Down Expand Up @@ -1231,9 +1229,8 @@ size_t ATHandler::write(const void *data, size_t len)
}
if (_debug_on && write_len < DEBUG_MAXLEN) {
if (write_len + ret < DEBUG_MAXLEN) {
debug_print((char *)data + write_len, ret);
debug_print((char *)data + write_len, ret, AT_TX);
} else {
debug_print(DEBUG_END_MARK, sizeof(DEBUG_END_MARK) - 1);
_debug_on = false;
}
}
Expand Down Expand Up @@ -1287,30 +1284,45 @@ void ATHandler::flush()
}
}

void ATHandler::debug_print(const char *p, int len)
void ATHandler::debug_print(const char *p, int len, ATType type)
Copy link
Contributor

Choose a reason for hiding this comment

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

I really don't like that we have this precedent of internal/external APIs, and which are alright to change in a patch, but so it goes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Especially since this a debug function.

{
#if MBED_CONF_CELLULAR_DEBUG_AT
if (_debug_on) {
#if MBED_CONF_MBED_TRACE_ENABLE
mbed_cellular_trace::mutex_wait();
#endif
char c;
for (ssize_t i = 0; i < len; i++) {
c = *p++;
if (!isprint(c)) {
if (c == '\r') {
debug("\n");
const int buf_size = len * 4 + 1; // x4 -> reserve space for extra characters, +1 -> terminating null
char *buffer = (char *)malloc(buf_size);
Copy link

Choose a reason for hiding this comment

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

Check for NULL or use nothrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

if (buffer) {
memset(buffer, 0, buf_size);

char *pbuf = buffer;
for (ssize_t i = 0; i < len; i++) {
const char c = *p++;
if (isprint(c)) {
*pbuf++ = c;
} else if (c == '\r') {
sprintf(pbuf, "<cr>");
pbuf += 4;
} else if (c == '\n') {
sprintf(pbuf, "<ln>");
pbuf += 4;
} else {
debug("#%02x", c);
sprintf(pbuf, "<%02X>", c);
pbuf += 4;
}
}
MBED_ASSERT((int)(pbuf - buffer) <= buf_size); // Check for buffer overflow

if (type == AT_RX) {
tr_info("AT RX (%2d): %s", len, buffer);
} else if (type == AT_TX) {
tr_info("AT TX (%2d): %s", len, buffer);
} else {
debug("%c", c);
tr_info("AT ERR (%2d): %s", len, buffer);
}

free(buffer);
} else {
tr_error("AT trace unable to allocate buffer!");
}
#if MBED_CONF_MBED_TRACE_ENABLE
mbed_cellular_trace::mutex_release();
#endif
}
#endif // MBED_CONF_CELLULAR_DEBUG_AT
}
Expand Down
7 changes: 6 additions & 1 deletion features/cellular/framework/AT/ATHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,12 @@ class ATHandler {
bool find_urc_handler(const char *prefix);

// print contents of a buffer to trace log
void debug_print(const char *p, int len);
enum ATType {
AT_ERR,
AT_RX,
AT_TX
};
void debug_print(const char *p, int len, ATType type);
};

} // namespace mbed
Expand Down