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

[rospy] Implement rospy.logXXX_throttle #812

Closed
wants to merge 4 commits into from

Conversation

wkentaro
Copy link
Contributor

Periodicall logging feature for python client.

"""
now = rospy.Time.now()

last_logging_time = self.last_logging_time_table.get(id)
Copy link
Member

@dirk-thomas dirk-thomas Jun 2, 2016

Choose a reason for hiding this comment

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

Is the id sufficient for this? From the following code I would expect both messages to be printed every three seconds (which I would assume is not the case currently?):

while True:
    loginfo_throttle(3, 'foo') or loginfo_throttle(3, 'foo')

Copy link
Contributor Author

@wkentaro wkentaro Jun 3, 2016

Choose a reason for hiding this comment

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

Maybe I need to get more detailed position where the function is called.
Currently, the inspect.stack()[1][1:] returns only line where the function is called:
Because there is the case where the logging functions, period and msg are all same.

% cat spam.py
import inspect


def loginfo_throttle():
    print(inspect.stack()[1])


def main():
    loginfo_throttle(); loginfo_throttle()


if __name__ == '__main__':
    main()

% python spam.py 
(<frame object at 0x7f3d6b2ed3a0>, 'spam.py', 9, 'main', ['    loginfo_throttle(); loginfo_throttle()\n'], 0)
(<frame object at 0x7f3d6b2ed3a0>, 'spam.py', 9, 'main', ['    loginfo_throttle(); loginfo_throttle()\n'], 0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But currently I don't know how to get the called location with information about lines and columns.
inspect.stack()[1][1:] only returns the line and line number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gavanderhoorn
Copy link
Contributor

gavanderhoorn commented Jun 3, 2016

Just a data point: I've looked into this some time in the past as well, and could not arrive at a solution that was both portable (inspect and / or sys._getframe() are not supported 'everywhere' and may not return the same information on all platforms / runtimes) and relatively straightforward to implement.

See rospy: throttling logmessage rate? for the question on ROS Answers and (my own) answer. I'm not really a Python expert, so it is very well possible that I missed something or am just ignorant.

It would be really nice if this could be fixed though, so +1 for working on it @wkentaro.

@wkentaro
Copy link
Contributor Author

wkentaro commented Jun 3, 2016

@gavanderhoorn In what cases, the inspect and sys._getframe() do not work? On windows?

@gavanderhoorn
Copy link
Contributor

I didn't say it doesn't work, I said it doesn't necessarily return the same information, and in the same format. I don't have my full notes anymore (it was 3 years ago), but iirc, some Python interpreters either don't return the required info, or don't return everything, or in a slightly different format. That makes it difficult to implement something that always works.

Now how much of a problem that is, I don't know. If we are happy with it working on the regular / official Python interpreters, fine by me. I just didn't want to implement something with if/else trees and / or special cases.

@wkentaro
Copy link
Contributor Author

wkentaro commented Jun 3, 2016

@dirk-thomas I have done the implementation. Could you please review again?

@dirk-thomas
Copy link
Member

As long as the caller_id is unique for every invocation of these new log functions this should be working. It tried several different way to "bypass" it but it worked flawless.

The caller_id doesn't have to be "the same" for different Python interpreters as long as the result is unique this should work.

Thanks for iterating on this. I have squashed and cherry-picked this in 5daef56. Can you please update the wiki page to add some documentation about these new functions mentioning that they are new in Kinetic and comment with a link to the diff here.

@dirk-thomas dirk-thomas closed this Jun 3, 2016
@wkentaro wkentaro deleted the logXXX-throttle branch June 3, 2016 16:13
@wkentaro
Copy link
Contributor Author

wkentaro commented Jun 3, 2016

Thanks for the review.
I updated the wiki: http://wiki.ros.org/action/diff/rospy/Overview/Logging?action=diff&rev1=18&rev2=19

BTW, Is that possible to add this feature also to Indigo and Jade?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants