Hello all, I have submitted a few logging related PRs to the core repos. I think I like the changes, but I have a nagging feeling they miss the point somehow. I wanted to get the community's opinion on these changes as well as start a discussion on minimizing backwards compatibility breakage. I plan to split this into two emails.
This email concerns this PR: https://github.com/apache/mynewt-core/pull/1196 This email is a bit easier than the next one, because I am mostly convinced that this change is good. The PR addresses some awkwardness in the logging API. Specifically, the old API requires the user to know that each log entry starts with a header, and to know the offset of the message body within a log entry. This burden is imposed on the user as follows: 1. log_append() - User supplied buffer must contain sufficient padding to accommodate a log entry header at the start (described in more detail here: https://github.com/apache/mynewt-core/issues/1173). 2. log_read() - The offset argument is relative to the start of the log entry, not to the start of the message body. Reading from offset 0 retrieves the header; reading from offset (0+hdr_size) retrieves the body. 3. log_walk() - Length passed to walk callback indicates the size of the entire entry, not just the size of the body. The callback has to adjust the length and offset to read the message body. I think these are all deficiencies in the logging API. The user should not need to know how log entries are structured and where the message body begins. However, I think the most important issue is the padding requirement of `log_append()`. The PR referenced above addresses these issues by introducing some new funtions: * log_append_body * log_append_mbuf_body * log_read_hdr * log_read_body * log_read_mbuf_body * log_walk_body The PR description goes into more detail about these functions. I think these functions are an improvement over the existing ones, because they are less error-prone without sacrificing any power (but please voice any disagreements). That said, I don't think it is necessary to break backwards compatibility. While the new function names are not quite ideal, it doesn't seem llike too much of a burden for users to include the "_body" suffix when accessing the log API. All comments welcome. Thanks, Chris
