Events type is bytes, not bytearray

[fcladera]

The decode_bytes prototype specifies events as bytes.

  void decode_bytes(
    const std::string & encoding, uint16_t width, uint16_t height, uint64_t timeBase,
    pybind11::bytes events);

but decoder.c tries to decode it as a bytearray. This can trigger the following issue with a simple EventArray message, that yields an immutable object for events.

>>> a = event_array_msgs.msg.EventArray()
>>> a
header:
  seq: 0
  stamp:
    secs: 0
    nsecs:         0
  frame_id: ''
height: 0
width: 0
seq: 0
time_base: 0
encoding: ''
is_bigendian: False
events: []
>>> type(a.events)
<class 'bytes'>

python3.8-dbg: /home/fclad/catkin_ws/src/event_array_py/src/decoder.cpp:35: 
void Decoder::decode_bytes(const string&, uint16_t, uint16_t, uint64_t, pybind11::bytes): Assertion `PyByteArray_Check(events.ptr())' failed.

This patch fixes the problem.

[berndpfrommer]

Fernando, thanks for contributing this patch. As you can see from the comments in the code you can clearly see that I didn't quite know what I was doing but just hacked it until it "worked". And in fact I'm using this code and it is giving me no errors. How exactly do you trigger the assertion above?

[fcladera]

Hi Bernd! Sorry for the sterile message last night, just wanted to submit it before I forget it 👎🏼

We are building our CI for the dataset and we triggered this issue when playing one bag. Ken was not able to replicate it in his computer, but we are using docker so I will come up with a container to replicate this.

[berndpfrommer]

Fernando, I appreciate the feedback!
I just created a branch with unit tests and I'm trying to figure out what's going on. I use decode_array() under ROS2, but they should both work. Give me some more time to look into decode_bytes() before you waste more time on it.

[fcladera]

My intuition is that the pybind11 version that comes with Focal is slightly older than the ros-noetic-pybind11-catkin.
pybind/pybind11#2799

fclad@m3ed-debug:~$ apt search pybind11
Sorting... Done
Full Text Search... Done
pybind11-dev/focal,focal,now 2.4.3-2build2 all [installed]
  seamless operability between C++11 and Python

pybind11-doc/focal,focal 2.4.3-2build2 all
  documentation for pybind11

python3-pybind11/focal,focal 2.4.3-2build2 all
  pybind11 helper module for Python 3

ros-noetic-pybind11-catkin/now 2.5.0-1focal.20210423.224835 amd64 [installed,local]

[berndpfrommer]

Alright, the idea of bytes vs bytearray was spot on! Now I can simply cast the pointer which saves another memory copy.
Please amend your PR like so:

 if (decoder) {
    decoder->setTimeBase(timeBase);
    decoder->setTimeMultiplier(1);  // report in usecs instead of nanoseconds                                                                                               
    delete cdEvents_;               // in case events have not been picked up                                                                                               
    cdEvents_ = new std::vector<EventCD>();
    delete extTrigEvents_;  // in case events have not been picked up                                                                                                       
    extTrigEvents_ = new std::vector<EventExtTrig>();
    // TODO(Bernd): use hack here to avoid initializing the memory                                                                                                          
    cdEvents_->reserve(maxSizeCD_);
    extTrigEvents_->reserve(maxSizeExtTrig_);
    const uint8_t * buf = reinterpret_cast<const uint8_t *>(PyBytes_AsString(events.ptr()));
    decoder->decode(buf, PyBytes_Size(events.ptr()), this);
  }

[fcladera]

Ok, this is great! It works really well in our end. Thank you so much for your help!