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

ceify the metro lib #470

Closed
trentgill opened this issue Feb 3, 2023 · 3 comments · May be fixed by #471
Closed

ceify the metro lib #470

trentgill opened this issue Feb 3, 2023 · 3 comments · May be fixed by #471
Milestone

Comments

@trentgill
Copy link
Collaborator

@tehn calling you for review!

i'm converting the metro lib to a C library with simple hooks. one thing i note is the crow docs have a more limited API than the norns docs for this module, but under the hood they actually use the same file.

my question is, do we need to support the (currently undocumented) features from norns? either way i'm going to push the whole thing into C land, but would love your opinion on what is necessary to provide as the API.

if we go with the full norns API, there are a couple parts that seem like under-the-hood details (aka not ever used by a script). specifically the "module control" functions here. and secondly, do we need the "module query" getters (which i note the metro.assigned seems to have incorrect docs, or at least i don't understand how it works).

it's a funny module to fixate on as i think it's utilized far less often since clock came to be, but still useful for a quick and dirty timer. i guess i'm interested to laser-focus on that quick & simple functionality without any extra fanfare -- contrasted with wanting to avoid any breakages in existing scripts. i guess a lot of this stuff just feels unnecessary in the tiny crow script context.

@trentgill trentgill added this to the 4.0 milestone Feb 3, 2023
@tehn
Copy link
Member

tehn commented Feb 4, 2023

definitely don't need module control --- and i really don't think we need module query either. those are both really for dynamic management which i can't see being useful in the crow context.

it's true, metro is really nice in that it's so simple.

@trentgill
Copy link
Collaborator Author

this is done in #471 which passes my basic testing of the published API (https://monome.org/docs/crow/reference/#metro) and a little more for direct setting of parameters of the clock with assignment syntax (metro[n].stage = 23 to set the current metro counter)

@trentgill
Copy link
Collaborator Author

Closing this issue as i've built & tested the ceified library. It could be merged if we need/want.

Secondly, the whole motivation for this ceification journey was perhaps unfounded. It was trying to solve out-of-memory issues thinking they were the cause of failing clocks / timelines. turns out that was not the issue (it was a bug in the clock.sync implementation), so i'm closing this issue as it doesn't actually fix anything directly.

of course we can continue this endeavour if we find we need to free up RAM in the runtime, but it's not a limiting factor right now, so i'm closing this.

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

Successfully merging a pull request may close this issue.

2 participants