Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.39%. Comparing base (b829450) to head (87d821a).

@@           Coverage Diff           @@
##           master   #14120   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         161      161           
  Lines       21200    21200           
=======================================
  Hits        20860    20860           
  Misses        340      340           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

@iabdalkader Is this streaming interface important for your use of the openamp module? If not, then this would be a low priority for us.

@iabdalkader Is this streaming interface important for your use of the openamp module? If not, then this would be a low priority for us.

No not at all, it's just something @jimmo asked for in the reviews. However you may want to look at the log issue, second commit.

However you may want to look at the log issue, second commit.

I had a look at this and see the problem. Can we fix that by using the proper logging interface, and calling metal_set_log_handler() and passing in a function that wraps mp_printf()?

Can we fix that by using the proper logging interface

Yes, I'll send it in a different PR.

Is this streaming interface important for your use of the openamp module?

Note the only blocker right now is #14082

I see that this now has mimxrt support, which is good, but it means that will be blocked on the streaming part of the API.

If you want to get the logging and/or mimxrt support merged faster then please put them in a separate PR.

If you want to get the logging and/or mimxrt support merged faster then please put them in a separate PR.

I kept them here in this PR and sent a new one for endpoint streaming #14181

There are some remaining DEBUG_printf in extmod/modopenamp_remoteproc_store.c that should probably be changed to metal_log() calls. Also extmod/modopenamp_remoteproc.c has a #define DEBUG_printf that's never used.

There are some remaining DEBUG_printf in extmod/modopenamp_remoteproc_store.c that should probably be changed to metal_log() calls. Also extmod/modopenamp_remoteproc.c has a #define DEBUG_printf that's never used.

I removed them all, as well as some debug_printf's in extmod/modopenamp.c that we've missed in the initial review. I think it's much better to control the whole log with a single switch.