Skip to content

add basic support for rt-thread#637

Merged
hathach merged 1 commit into
hathach:masterfrom
tfx2001:master
Feb 8, 2021
Merged

add basic support for rt-thread#637
hathach merged 1 commit into
hathach:masterfrom
tfx2001:master

Conversation

@tfx2001
Copy link
Copy Markdown
Contributor

@tfx2001 tfx2001 commented Feb 7, 2021

Describe the PR
Add a basic support for RT-Thread RTOS. More about the RT-Thread.

Additional context
None.

Repository owner deleted a comment from Tagore-git Feb 7, 2021
Copy link
Copy Markdown
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR lack define value for OPT_OS_RTTHREAD. Is it easy enough to add cdc_msc_rtthread example so that we could avoid building issue like this for future changes.

@tfx2001
Copy link
Copy Markdown
Contributor Author

tfx2001 commented Feb 8, 2021

It is my mistake.

I'm a student developer in university, and I am still studying embedded development. Maybe add cdc_msc_rtthread example is a little hard for me. I'm concerned about overlooking support for other MCUs.

In fact, the RT-Thread team already provides a USB Device stack in RT-Thread, but not in RT-Thread Nano (the RTOS kernel of RT-Thread). I hope to provide more options for Chinese developers.

Thank you for your review.

@hathach
Copy link
Copy Markdown
Owner

hathach commented Feb 8, 2021

no problem at all if you think adding the example is not trivial, though have you been able to run rt-thread with tinyusb successfully on their studio IDE ?

@tfx2001
Copy link
Copy Markdown
Contributor Author

tfx2001 commented Feb 8, 2021

Yes. I have tested it in RT-Thread Studio, I've also tested it in my own CMake project with RT-Thread Nano. They all work fine.

Copy link
Copy Markdown
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, It looks good. There is only a request to suppress unused warning and a bit of code style. Afterwards this should be good for the merge

Comment thread src/osal/osal_rtthread.h
Comment thread src/osal/osal_rtthread.h
Comment thread src/osal/osal_rtthread.h Outdated
Comment on lines +94 to +95
osal_queue_def_t _name = { \
.depth = _depth, .item_sz = sizeof(_type), .buf = _name##_##buf};
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please merge this 2 line together, also needs a space between ##buf and closing }

@hathach
Copy link
Copy Markdown
Owner

hathach commented Feb 8, 2021

Yes. I have tested it in RT-Thread Studio, I've also tested it in my own CMake project with RT-Thread Nano. They all work fine.

That is good enough for this PR

Copy link
Copy Markdown
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look good, thanks for the PR. Will merge when ci complete

@hathach hathach merged commit ecd16cf into hathach:master Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants