Skip to content

Conversation

@fedeoli
Copy link
Collaborator

@fedeoli fedeoli commented Nov 16, 2024

Moving BaseSensorBackend from base_backend to sensor_backend. BaseSensorBackend is a base class for sensor backends. It includes methods for initializing sensor parameters, fetching data (get_data), transforming data into desired reference frames (transform), and reading sensor data (read_sensor). The read_sensor method checks if data transformation is needed based on frame references.

Questions:

  • does it make sense as a structure? I was thinking of adding different backends for specific sensors inheriting from BaseSensorBackend, for instance, creating a CameraSensorBackend going into the details of how generic cameras work, and then finalizing the implementation in the pybullet_backend file.
  • are the features in SensorBaseBackend general enough? Can you think of something else?
  • Comments on the coding? Specifically, the read_sensor method should be an abstract interface depending on more sensor-dependent actions (get_data, transform). Does it make sense to use abstractmethod do delegate the actual implementation to lower-level classes?

@fedeoli fedeoli requested a review from omershalev November 16, 2024 10:11
@fedeoli fedeoli self-assigned this Nov 16, 2024
@omershalev
Copy link
Collaborator

General comment so far: we should exclude various files from Git. This includes the .vscode folder and all .pyc files. These need to be included in the .gitignore file (the ones already uploaded to Github need to be manually deleted).

from abc import ABC, abstractmethod


class BaseSensorBackend(ABC):
Copy link
Collaborator

Choose a reason for hiding this comment

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

With regards to ABC - that is indeed the way people do it in Python, though it isn't necessary to use it.
I personally refrained from using it thus far, but it isn't necessarily the right thing to do.
Do you know the pros and cons for using it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, not really; I just found it a good answer to the question: what if I know there will be this method in the children, but it will be different in each of them?

Also, how can you avoid using it? Just with a pass or a raise.NotImplementedError() ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, raise NotImplementedError and pass indeed.
ABC is probably the better recommended way so let's go with it, I'll read a little more about it.

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.

3 participants