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

Ouster lidar - log UDP #990

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Ouster lidar - log UDP #990

wants to merge 6 commits into from

Conversation

tajgr
Copy link
Collaborator

@tajgr tajgr commented Apr 16, 2024

This is a first attempt to log data from Ouster lidar. Only raw UDP packet now.
There is a new modification of the HTTP driver which allows POST method and "two side" communication.

@tajgr tajgr requested a review from m3d April 16, 2024 18:26
if isinstance(out_data, list):
assert len(out_data) == 3, out_data # unsupported output data
url, header, data = out_data
request = urllib.request.Request(url, data=data, headers=header, method='POST')
Copy link
Member

Choose a reason for hiding this comment

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

I would keep the original Request names, i.e. headers everywhere

else:
assert False, out_data # unsupported output data

with urllib.request.urlopen(request, timeout=0.5) as f:
Copy link
Member

Choose a reason for hiding this comment

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

the timeout should be part of module config, and 0.5s maybe as default? something like request_timeout_sec??

self.input_thread = Thread(target=self.run_input, daemon=True)
bus.register('raw', 'response')
self.url = config.get('url')
self.output = config.get('output', False)
Copy link
Member

Choose a reason for hiding this comment

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

is not url and output completely independent? If yes then they do not have to be in one common module or what is the motivation to keep it together?

from osgar.node import Node


class OusterLidarDummy(Node):
Copy link
Member

Choose a reason for hiding this comment

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

why Dummy?

assert not self.configuration_saved # The configuration should be delivered only once.
if self.verbose:
print(data)
self.publish("lidar_config", data.decode())
Copy link
Member

Choose a reason for hiding this comment

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

what is the data you receive? does not decode need encoding?

@@ -175,18 +175,29 @@ def _send(self, data):

class LogHTTP:
Copy link
Member

Choose a reason for hiding this comment

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

maybe add new class LogHTTPRequest?

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