Skip to content

ports/mimxrt:Add thread support. #13755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tonyzhangnxp
Copy link

@tonyzhangnxp tonyzhangnxp commented Feb 26, 2024

Adds thread support on ports/mimxrt, mimicks it from stm32.

@tonyzhangnxp tonyzhangnxp marked this pull request as ready for review February 26, 2024 08:17
@robert-hh
Copy link
Contributor

A while ago I implemented threading for the MIMXRT port because someone had asked for it. But I did not make a PR. There were too many PR open already, and threading is a rarely asked feature compared to asyncio. Some comment about this PR:

  • I did not enable threading for the MIMXRT 10xx boards, since these MCUs are low on RAM.
  • You do not have to set/unset threading in each board's .mk file. You can do that in Makefile.

@tonyzhangnxp
Copy link
Author

Hello Robert

Thanks for your time. Some customers requested the threading feature in micropython on MIMXRT platform.
It's a good idea that this feature can be merged into the main branch.

  • I did not enable threading for the MIMXRT 10xx boards, since these MCUs are low on RAM.
    [Tony] Which one has low memory, do you mean MIMXRT1010 evk ? which has 128KB RAM, it's not enough for threading ?
  • You do not have to set/unset threading in each board's .mk file. You can do that in Makefile.
    [Tony] In my understanding, mk file is used to dis/enable the feature, so I added it for all the boards. If add it in Makefile, that means threading is default enable for all the boards?

@robert-hh
Copy link
Contributor

which has 128KB RAM, it's not enough for threading

Of the 128k only 64 k are available for the OCRM section, which holds the MicroPython heap. The stack of threads is taken from the heap, like all other dynamic data for Python scripts.

If add it in Makefile, that means threading is default enable for all the boards?

You can enable it if not already defines otherwise in the board's .mk file. May be fewer files changed.

@tonyzhangnxp
Copy link
Author

hello
Set MICROPY_PY_THREAD = 1 if undefined in mpconfigport.h, and set MICROPY_PY_THREAD=0 in mk of mimxrt1010 platforms.

@robert-hh
Copy link
Contributor

Thanks, well noticed. The interesting information is, that there are customers using MicroPython. Do you know why they did not ask at the MicroPython site?

@robert-hh
Copy link
Contributor

The test coverage for tests/thread is fine. The same as Pyboard:

25 tests performed (139 individual testcases)
24 tests passed
5 tests skipped: mutate_bytearray mutate_dict mutate_instance mutate_list mutate_set
1 tests failed: thread_ident1

@tonyzhangnxp
Copy link
Author

Thanks, well noticed. The interesting information is, that there are customers using MicroPython. Do you know why they did not ask at the MicroPython site?

As I know the customer is using MicroPython in some industrial products, and they need quick support, so the device vendor is the first option.
I thought maybe merging this feature in the main branch should be good for others.

@projectgus
Copy link
Contributor

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

Signed-off-by: tonyzhangnxp <tony.zhang@nxp.com>
@tonyzhangnxp
Copy link
Author

All STATIC had been replaced with static

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants