nxp-mcuxpresso / rpmsg-lite

RPMsg implementation for small MCUs
BSD 3-Clause "New" or "Revised" License
235 stars 74 forks source link

Bug/env threadx queue #37

Closed TimTTP closed 1 year ago

TimTTP commented 1 year ago

Removed incorrect struct keyword and unnecessary casting. This matches use of TX_QUEUE in other parts of the file so is probably just a mistake that was left over from a previous change or update to the ThreadX API.

It is using struct TX_QUEUE but TX_QUEUE is actually a typedef from tx_api.h:

typedef struct TX_QUEUE_STRUCT {
  ...
} TX_QUEUE;

Found it when compiling for NXP RT1170 chip based on rpmsg-lite examples ported to ThreadX.

Before change:

arm-none-eabi-gcc [ ... lots of arguments ... ] "../rpmsg_lite/rpmsg_lite/porting/environment/rpmsg_env_threadx.c"
../rpmsg_lite/rpmsg_lite/porting/environment/rpmsg_env_threadx.c: In function 'env_create_queue':
../rpmsg_lite/rpmsg_lite/porting/environment/rpmsg_env_threadx.c:621:67: error: invalid application of 'sizeof' to incomplete type 'struct TX_QUEUE'
  621 |     queue_ptr       = (struct k_msgq *)env_allocate_memory(sizeof(struct TX_QUEUE));
      |                                                                   ^~~~~~

After change there are no warnings or errors.

I've compiled it with MCU Xpresso and run with my board and it all seems to work.

Please let me know what you think and if this MR is useful or needs any more work to be merged in.

Thanks,

Tim TTP

MichalPrincNXP commented 1 year ago

Hello @TimTTP , thank you for reporting that issue. In fact, I have already found that recently and got the fix ready together with other threadx related fixes for the next rpmsg_lite release. May I ask you to have a try attached updated source in your project if all is ok, please? Thank you Michal rpmsg_env_threadx.zip

TimTTP commented 1 year ago

Hi Michal, thanks for getting back to me!

Sounds great - the update seems to work well, I just had to update the header file declaration for env_get_queue and env_put_queue because the timeout parameter changed to uintptr_t, but I presume that is all included in your update.

Feel free to close this MR if it's no longer useful.

When are you planning on releasing the next update?

Thanks,

Tim

MichalPrincNXP commented 1 year ago

Hello Tim, thanks for trying on your side and confirming it is ok now. I plan to provide the update soon, the next official release is planned at the end of July. If you are not against I will close this PR, ok? Regards Michal

TimTTP commented 1 year ago

Thanks Michal, yea that's fine 👍