OpenAMP / open-amp

The main OpenAMP library implementing RPMSG, Virtio, and Remoteproc for RTOS etc
https://www.openampproject.org/
Other
678 stars 278 forks source link

openamp: add error log when ept->cb return error #519

Open CV-Bowen opened 8 months ago

CV-Bowen commented 8 months ago

Add helpful log when rpmsg endpoint callback return error.

CV-Bowen commented 8 months ago

@arnopo Hi, why the docs/readthedocs is always failed. Did I miss some things to do?

arnopo commented 8 months ago

@arnopo Hi, why the docs/readthedocs is always failed. Did I miss some things to do?

you should rebase your work on main branch, seems that your 6 commits behind

CV-Bowen commented 8 months ago

@arnopo Thanks, now CI passed.

xiaoxiang781216 commented 8 months ago

@arnopo how about this patch?

arnopo commented 8 months ago

@arnopo how about this patch?

we have a duplication with the assert and the error message. The question is what is the expected behavior: Crash on error, just display an error message or both?

Another question is why this error message is not displayed in the callback itself? Seems to me that this would be more efficient for the debug. And the user can decide to print an error and return a status 0 to not crash the system.

an alternative could be to update the RPMSG_ASSERT macro to have variadic arguments, something like this ( not tested)

#define RPMSG_ASSERT(_exp, format, ...) do { \
        if (!(_exp)) { \
            metal_log(METAL_LOG_EMERGENCY, \
                  "FATAL: %s - " format,  __func__, ##__VA_ARGS__); \
            metal_assert(_exp); \
        } \
    } while (0)

but sound to me better to display the message in the endpoint callback

CV-Bowen commented 7 months ago

@arnopo OK, maybe add error log to the endpoint callback is better although it needs more works.

github-actions[bot] commented 5 months ago

This pull request has been marked as a stale pull request because it has been open (more than) 45 days with no activity.