erpc icon indicating copy to clipboard operation
erpc copied to clipboard

[BUG] Cursor class is inside message buffer class

Open amgross opened this issue 3 years ago • 3 comments

Describe the bug

1Cursor class is in side message buffer class, although the message buffer don't use its inner cursor and the cursor is using message buffer that it gets as input instead its own message buffer. It makes things unclear and other bugs (In other issues I will open)

To Reproduce

Expected behavior

there are three options (from better to worth in my opinion):

  1. instead of cursor class, to have in the message buffer variable of m_cursor_pos (That is what I suggesting)
  2. Have the cursor class be separated
  3. Cursor class will use the message class it is inside of.

Screenshots

Desktop (please complete the following information):

  • OS:
  • eRPC Version:v1.9.0

Steps you didn't forgot to do

  • [X] I checked if there is no related issue opened/closed.
  • [X] I checked that there doesn't exist opened PR which is solving this issue.

Additional context

amgross avatar Jun 06 '22 10:06 amgross

@MichalPrincNXP @Hadatko any thoughts?

amgross avatar Jun 08 '22 04:06 amgross

HI @amgross thank you for all your inputs. I think that what you proposed here https://github.com/EmbeddedRPC/erpc/issues/282 will work nice. We can remove m_remainig and use MessageBuffer variables which can be accessed already like getLength() for write and getUsed for read.

Here it is your 3. bullet. I think i like it a bit more than 1. bullet.

Hadatko avatar Jun 15 '22 05:06 Hadatko

OK, I can live with bullet 3. I prefer instead of uint8_t *m_pos; to have uint16_t m_offset;, I think it is better to have just one pointer to the buffer and everyone will take offset from it, what do you think? Also if we want to keep the nested classes, we can use m_buffer->m_used instead of m_buffer->getUsed()

amgross avatar Jun 15 '22 08:06 amgross

https://github.com/EmbeddedRPC/erpc/pull/378 ;)

Hadatko avatar Sep 27 '23 13:09 Hadatko