search

wavelab / ximea_ros_cam

Ximea ROS Camera
MIT License
14 stars 12 forks source link

Updates #43

Open shaun-createc opened 4 years ago

shaun-createc commented 4 years ago
carloswanguw commented 4 years ago

I will review this tonight or within this week, thanks

shaun-createc commented 4 years ago

Hi Carlos(?), I'm hoping to use this driver in some up and coming projects. I like that is a nodelet and it looks like you have spent some time getting the settings right. I hope you don't mind the refactoring.

With adding more checks I've introduced a few possible bugs. For cameras that do not support XI_PRM_ACQ_TIMING_MODE and XI_PRM_LIMIT_BANDWIDTH_MODE like the MQ the driver will now show an error when limiting frame rate and a warn when limiting bandwidth. In both cases I think this is a false negative. When I get the chance next week I'll alter it to allow these calls to fail without reporting a big red error.

From: carloswanguw notifications@github.com Sent: 06 February 2020 16:10 To: wavelab/ximea_ros_cam ximea_ros_cam@noreply.github.com Cc: Shaun Prickett shaun.prickett@createc.co.uk; Author author@noreply.github.com Subject: Re: [wavelab/ximea_ros_cam] Updates (#43)

I will review this tonight or within this week, thanks

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/wavelab/ximea_ros_cam/pull/43?email_source=notifications&email_token=AEBXA35S7MZUDCRXDCNIF5DRBQZA7A5CNFSM4KQ7PN62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK7ZBIA#issuecomment-582979744, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AEBXA33ONN6DEJQMGRHYLZTRBQZA7ANCNFSM4KQ7PN6Q.

Shaun Prickett +44 (0) 1900 828112 [Createc]http://createc.co.uk Unit 4, Derwent Mills Commercial Park Cockermouth, Cumbria, CA130HT [createc] The content of this message is confidential. If you have received it by mistake, please inform us by an email reply and then delete the message. It is forbidden to copy, forward, or in any way reveal the contents of this message to anyone. The integrity and security of this email cannot be guaranteed over the Internet. Therefore, the sender will not be held liable for any damage caused by the message. Registered Name: Create Technologies Limited, Registered office: Unit 4, Derwent Mills, Cockermouth, CA13 0HT, United Kingdom. Registered Number: 07293648. Registered in England and Wales. www.createc.co.uk.

carloswanguw commented 4 years ago

An additional comment will be to do some timing tests (start the camera at a low FPS, then a high FPS) and vary the CPU load in the computer to emulate variance in the timestamp acquisition period. Plot the camera timestamps + ROS timestamps and see if the estimation makes sense or if there are any edge cases we might not have accounted for (aside from the wraparound)

shaun-createc commented 4 years ago

Do you have any objections to bringing in some C++11 features? I'm finding the limits of the ros Time types frustrating, and I would like to use chrono.

carloswanguw commented 4 years ago

Do you have any objections to bringing in some C++11 features? I'm finding the limits of the ros Time types frustrating, and I would like to use chrono.

Feel free to add chrono, I don't have any objections to any standard lib that comes with C++11

shaun-createc commented 4 years ago

Thanks for all the useful comments. I need to jump onto something else for a few days. I dealt with a bug yesterday with the frame limit handling on certain models and I added the ability to suppress the warn to the get set functions. This for when we know the call might fail. I had a chance to simulate a 33 bit nanosecond stamp where I found the ros::Time types throw, the overflow handling needs more work, which I'll try to fit in this week (camera is on loan)

carloswanguw commented 4 years ago

Thanks for all the useful comments. I need to jump onto something else for a few days. I dealt with a bug yesterday with the frame limit handling on certain models and I added the ability to suppress the warn to the get set functions. This for when we know the call might fail. I had a chance to simulate a 33 bit nanosecond stamp where I found the ros::Time types throw, the overflow handling needs more work, which I'll try to fit in this week (camera is on loan)

No problem, just continue with the updates and I will continue to review when I have the time