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

Runtime logging configuration #1127

Merged
merged 4 commits into from Nov 22, 2016

Conversation

@sergeuz
Copy link
Member

commented Sep 23, 2016

This PR adds support for runtime logging configuration, which allows to enable logging on already running system via USB control requests. Some background on the topic can be found here: #1037. Note that this PR is based on the common code from another branch which is being reviewed here: #1126

Parameters that can be configured dynamically: destination stream (Serial, Serial1, USBSerial1), message format (plain text, JSON), message filtering (level and category-based).

This PR also adds basic tools for acceptance testing, such as handling of serial IO, USB requests, application flashing, etc. The test utilizing above functionality can be found here.

I'll be adding further comments directly in the code to provide more details on the implementation.


Doneness:

  • Contributor has signed CLA
  • Problem and Solution clearly stated
  • Code peer reviewed
  • API tests compiled
  • Run unit/integration/application tests on device
  • Add documentation
  • Add to CHANGELOG.md after merging (add links to docs and issues)

Features

  • Added support for runtime logging configuration, which allows to enable logging on already running system via USB control requests.
@sergeuz sergeuz referenced this pull request Sep 24, 2016
7 of 7 tasks complete
@@ -78,20 +78,21 @@ void log_set_callbacks(log_message_callback_type log_msg, log_write_callback_typ
}

void log_message_v(int level, const char *category, LogAttributes *attr, void *reserved, const char *fmt, va_list args) {
if (!log_msg_callback && (!log_compat_callback || level < log_compat_level)) {
const log_message_callback_type msg_callback = log_msg_callback;

This comment has been minimized.

Copy link
@sergeuz

sergeuz Sep 24, 2016

Author Member

Current application callback is copied to a local variable, because LogManager may reset it at any time in multithreaded configuration (turning low-level logging functions to no-op), e.g. when last log handler has been unregistered.

Task* availTask_; // Task pool
Task* firstTask_; // Task queue
Task* lastTask_;
};

This comment has been minimized.

Copy link
@sergeuz

sergeuz Sep 24, 2016

Author Member

This class implements a simple callback queue where callbacks can be enqueued from an ISR and then invoked asynchronously by a regular thread. The implementation doesn't use os_queue_t and can be used on platforms without concurrency support. Currently, there's a single queue for ISR tasks, and it's processed by the system thread as part of the primary event loop.

size_t request_size; // Request size
size_t reply_size; // Reply size (initialized to 0)
int format; // Data format (as defined by DataFormat enum)
} USBRequest;

This comment has been minimized.

Copy link
@sergeuz

sergeuz Sep 24, 2016

Author Member

This structure describes an asynchronous USB request. A request handler processes request data using data and request_size fields, then optionally stores reply data to the same buffer, sets reply_size field accordingly and calls system_set_usb_request_result() function to finalize processing.

typedef bool(*usb_request_app_handler_type)(USBRequest* req, void* reserved);

// Sets application callback for USB requests
void system_set_usb_request_app_handler(usb_request_app_handler_type handler, void* reserved);

This comment has been minimized.

Copy link
@sergeuz

sergeuz Sep 24, 2016

Author Member

This function allows to set a callback that will be invoked for requests that should be processed in application thread. In current implementation such requests are USB_REQUEST_LOG_CONFIG and USB_REQUEST_CUSTOM.

~USBRequestData();
};

USBRequestData usbReq_;

This comment has been minimized.

Copy link
@sergeuz

sergeuz Sep 24, 2016

Author Member

Only one active asynchronous USB request at a time is supported.

protected:
virtual void logMessage(const char *msg, LogLevel level, const char *category, const LogAttributes &attr) override;
virtual void write(const char *data, size_t size) override;
};

This comment has been minimized.

Copy link
@sergeuz

sergeuz Sep 24, 2016

Author Member

This class implements JSON-formatted output for log messages to simplify machine processing.

virtual LogHandler* createHandler(const char *type, LogLevel level, LogCategoryFilters filters, Print *stream,
const JSONValue &params) = 0; // TODO: Use some generic container or a buffer instead of JSONValue
virtual void destroyHandler(LogHandler *handler);
};

This comment has been minimized.

Copy link
@sergeuz

sergeuz Sep 24, 2016

Author Member

Handler and stream factory classes are extension points of the logging framework and allow to dynamically configure custom log handlers in the same way as built-in ones. This feature is also used for unit testing.

};

typedef FactoryDeleter<LogHandler, LogHandlerFactory, &LogHandlerFactory::destroyHandler> LogHandlerFactoryDeleter;
typedef FactoryDeleter<Print, OutputStreamFactory, &OutputStreamFactory::destroyStream> OutputStreamFactoryDeleter;

This comment has been minimized.

Copy link
@sergeuz

sergeuz Sep 24, 2016

Author Member

These custom deleters are used with std::unique_ptr to simplify error handling.

}
const char* const name = category + pos;
// Use binary search to find existent node or position for new node
}

This comment has been minimized.

Copy link
@sergeuz

sergeuz Sep 24, 2016

Author Member

This code has been refactored to reduce code duplication.

#if PLATFORM_ID != 3
if (stream == &Serial) {
// Uninitializing primary USB serial causes the device to get disconnected from the host
// Serial.end();

This comment has been minimized.

Copy link
@sergeuz

sergeuz Sep 24, 2016

Author Member

Fixed by this PR: #1122

@sergeuz sergeuz force-pushed the feature/usb_logging_2_of_2 branch from fe2ad9f to 1e04ef7 Sep 29, 2016

@sergeuz sergeuz added this to the 0.7.x milestone Oct 8, 2016

;;
electron)
USB_VENDOR_ID=2b04
USB_PRODUCT_ID=d00a

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Oct 16, 2016

Member

This should be c00a (otherwise it targets Electron in DFU mode)

This comment has been minimized.

Copy link
@sergeuz

sergeuz Oct 16, 2016

Author Member

Thanks! Honestly, I don't remember from where I copied these IDs. Since you're working with this branch already, would you mind committing correct IDs?

;;
p1)
USB_VENDOR_ID=2b04
USB_PRODUCT_ID=d008

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Oct 16, 2016

Member

This should be c008 (otherwise it targets P1 in DFU mode)

@sergeuz sergeuz force-pushed the feature/usb_logging_1_of_2 branch 2 times, most recently from 0397166 to b1d7d74 Oct 16, 2016

@sergeuz sergeuz force-pushed the feature/usb_logging_2_of_2 branch from 1e04ef7 to 6bf20b5 Oct 22, 2016

@sergeuz sergeuz force-pushed the feature/usb_logging_1_of_2 branch from b1d7d74 to 7d404a8 Oct 29, 2016

@sergeuz sergeuz force-pushed the feature/usb_logging_2_of_2 branch from 6bf20b5 to cbda226 Oct 29, 2016

@avtolstoy avtolstoy referenced this pull request Oct 30, 2016
6 of 7 tasks complete

@technobly technobly self-assigned this Nov 21, 2016

@technobly
Copy link
Member

left a comment

💥

  • user/tests/accept/init_env:41 and 45 should be c00_ (fixed in #1153)
  • wiring/src/spark_wiring_logging.cpp:623 can be uncommented after PR 1122 is merged

@technobly technobly merged commit f835972 into feature/usb_logging_1_of_2 Nov 22, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@technobly technobly deleted the feature/usb_logging_2_of_2 branch Nov 22, 2016

@technobly technobly removed their assignment Nov 29, 2016

@avtolstoy avtolstoy referenced this pull request Nov 29, 2016
6 of 7 tasks complete

@technobly technobly modified the milestones: 0.7.x, 0.6.1 Nov 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.