search

zivid / zivid-ros

Official ROS driver for Zivid 3D cameras
BSD 3-Clause "New" or "Revised" License
60 stars 45 forks source link

Add support for normals #57

Closed apartridge closed 2 years ago

apartridge commented 2 years ago

I have bumped minimum required SDK with 2.5. But it is also possible to support older Zivid SDK with ifdef's to disable the normals-related code.

I am thinking it is fine to bump minimum SDK version now, even though in general we want to keep compatibility with older SDK's as long as possible. Customers on older SDK must pick an older version from the Releases tab.

runenordmo commented 2 years ago

I have bumped minimum required SDK with 2.5. But it is also possible to support older Zivid SDK with ifdef's to disable the normals-related code.

I am thinking it is fine to bump minimum SDK version now, even though in general we want to keep compatibility with older SDK's as long as possible. Customers on older SDK must pick an older version from the Releases tab.

What will decide when we use an ifdef instead of bumping the version? I think we should have some guidelines for this, @apartridge .

What if we later add a feature to the ROS driver that is very useful also for those on SDK in range [2.0, 2.4.2]?

apartridge commented 2 years ago

I have bumped minimum required SDK with 2.5. But it is also possible to support older Zivid SDK with ifdef's to disable the normals-related code. I am thinking it is fine to bump minimum SDK version now, even though in general we want to keep compatibility with older SDK's as long as possible. Customers on older SDK must pick an older version from the Releases tab.

What will decide when we use an ifdef instead of bumping the version? I think we should have some guidelines for this, @apartridge .

What if we later add a feature to the ROS driver that is very useful also for those on SDK in range [2.0, 2.4.2]?

  • What will be their way of obtaining those features?

For now we will bump the minimum required SDK version to 2.5. If we add new features to ROS wrapper later that we want to give for pre-2.5 users, they would need to cherry-pick the changes, or comment out the normals code. We could also add ifdefs at a later point ourselves.

runenordmo commented 2 years ago

I have bumped minimum required SDK with 2.5. But it is also possible to support older Zivid SDK with ifdef's to disable the normals-related code. I am thinking it is fine to bump minimum SDK version now, even though in general we want to keep compatibility with older SDK's as long as possible. Customers on older SDK must pick an older version from the Releases tab.

What will decide when we use an ifdef instead of bumping the version? I think we should have some guidelines for this, @apartridge . What if we later add a feature to the ROS driver that is very useful also for those on SDK in range [2.0, 2.4.2]?

  • What will be their way of obtaining those features?

For now we will bump the minimum required SDK version to 2.5. If we add new features to ROS wrapper later that we want to give for pre-2.5 users, they would need to cherry-pick the changes, or comment out the normals code. We could also add ifdefs at a later point ourselves.

That sounds good to me :+1: