Skip to content
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

Restore original prototype of mp_sched_schedule #252

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

amirgon
Copy link

@amirgon amirgon commented Dec 16, 2018

This is important for compatability with libraries that call mp_sched_schedule and need to compile on both the original Microptython and lobo's Micropython. New prototype name changed to mp_sched_schedule_ex

…for compatability with libaries that call `mp_sched_schedule` and need to compile on both the original Microptython and lobo's Micropython. New prototype name changed to `mp_sched_schedule_ex`
@loboris
Copy link
Owner

loboris commented Dec 17, 2018

There are reasons why the mp_sched_schedule was changed.
If writing the C-module wich uses mp_sched_schedule for both original MicroPython and this port, it should be easy to conditionally compile for one or the other, something like this

#if MICROPY_PY_SYS_PLATFORM_LOBO
#define mymp_sched_schedule(x) mp_sched_schedule(x, NULL);
#else
#define mymp_sched_schedule(x) mp_sched_schedule(x);
#endif

then use mymp_sched_schedule() instead of mp_sched_schedule in your module.
It is simple and should work.

@amirgon
Copy link
Author

amirgon commented Dec 17, 2018

@loboris
I understand there are reasons for the change, that is why I kept your version as mp_sched_schedule_ex.
However, in my opinion it's cleaner to keep the same API, instead of adding #ifs and changing all calls to mp_sched_schedule.

With my PR, when you have a Micropython library that uses mp_sched_schedule it would work without a change on your branch. With your suggestion, such library would have to be adapted by changing each mp_sched_schedule call to mymp_sched_schedule.

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.

None yet

2 participants