Skip to content

Updates for global severity threshold -> default severity threshold#130

Closed
dhood wants to merge 2 commits intomasterfrom
logging_config
Closed

Updates for global severity threshold -> default severity threshold#130
dhood wants to merge 2 commits intomasterfrom
logging_config

Conversation

@dhood
Copy link
Copy Markdown
Member

@dhood dhood commented Oct 19, 2017

connects to ros2/rcutils#57

This is the minimum PR required to keep rclpy working with the changes in ros2/rcutils#57

It does not attempt to leverage any of the configuration of loggers (that'll be a separate PR)

@dhood dhood self-assigned this Oct 19, 2017
@dhood dhood added the in progress Actively being worked on (Kanban column) label Oct 19, 2017
* \return severity
*/
static PyObject *
rclpy_logging_get_severity_threshold(PyObject * Py_UNUSED(self), PyObject * Py_UNUSED(args))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should the name of this function be changed accordingly?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, there's a conceptual adaptation of the rclpy use of rcutils logging coming in another rclpy PR, but this PR is just the bare minimum to keep it compiling if ros2/rcutils#57 gets merged (so the PRs don't block each other)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds good to me.

INFO = 20
WARN = 30
ERROR = 40
FATAL = 50
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we actually get the values from rcutils/logging.h rather than redefining them here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, we should, eventually. in my mind that would be done by moving the values into the rcutils.logging python module so we can include them here, and empy template them into rcutils' logging.h. Since there's a lot going on in ros2/rcutils#57 already, I think it's best to put it off to a separate step

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds good to me.

@dhood
Copy link
Copy Markdown
Member Author

dhood commented Nov 9, 2017

closed in favour of #132

@dhood dhood closed this Nov 9, 2017
@dhood dhood deleted the logging_config branch November 9, 2017 19:25
@dhood dhood removed the in progress Actively being worked on (Kanban column) label Nov 9, 2017
YuanYuYuan pushed a commit to YuanYuYuan/rclpy that referenced this pull request Nov 12, 2025
…urce

Added defaults to MD5SUM and DEFINITION
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants