-
Notifications
You must be signed in to change notification settings - Fork 12
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
Logging of raw laser data + fixup for turning off in navpat #34
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please not that I have not actually tried to run the code.
self._buffer = "" | ||
Laser.__init__( self, **kw ) | ||
self.sendCmd( 'sMN SetAccessMode 03 F4724744' ) | ||
|
||
def __del__( self ): | ||
self.socket.close() | ||
self.raw_log.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__del__
is not guaranteed to be called. The only way to be sure close()
is really called is to use context manager. It might not cause problem in this particular case because you call flush after each write but in general I would not consider this to be a good practice. For the future please consider using context managers for such cases.
@@ -69,6 +69,7 @@ def attach_sensor(robot, sensor_name, metalog): | |||
function2 = SourceLogger(None, remission_log_name).get | |||
else: | |||
robot.laser = LaserIP(remission=True) | |||
robot.laser.stopOnExit = False # for faster boot-up |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand the naming here - stopOnExit
? If it is False
something should not stop on exit. That something is laser. So, is stop actually a shutdown? And exit is end of the program? I don't have a much better name though 😟
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is on sys.exit() of the program - it is rather "legacy" variable (taken from Eduro).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you planning to handle somehow the act of shutting down everything? I remember one of the problems was not waiting for the threads to really finish/exit.
This code block will be replaced by newer logger (from https://github.com/robotika/naio), so I will merge it to simplify also Python3 transition. |
- add timestamp into laserraw binary log - add tool for parsing raw laser data
This is the second pull request from recent feature/testYYMMDD related to laser logging and debugging issue with 20s gaps. They were caused by previously turned off laser, but detailed logging of communication could be useful also in the future (and now I would need it also for Velodyne, so some kind of reuse is expected).