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

DEL.X / DEL.R ops #164

Merged
merged 11 commits into from
Oct 15, 2018
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

- **NEW**: new op: DEVICE.FLIP
- **FIX**: [some keyboards losing keystrokes](https://github.com/monome/teletype/issues/156)
- **NEW**: new op: DEL.X
- **NEW**: new op: DEL.R
- **IMP**: DELAY_SIZE increased to 16 from 8

## v3.0

Expand Down
18 changes: 17 additions & 1 deletion docs/ops/delay.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ prototype = "DEL x: ..."
short = "Delay command by `x` ms"
description = """
Delay the command following the colon by `x` ms by placing it into a buffer.
The buffer can hold up to 8 commands. If the buffer is full, additional commands
The buffer can hold up to 16 commands. If the buffer is full, additional commands
will be discarded.
"""
["DEL.CLR"]
Expand All @@ -12,3 +12,19 @@ short = "Clear the delay buffer"
description = """
Clear the delay buffer, cancelling the pending commands.
"""
["DEL.X"]
prototype = "DEL.X x y: ..."
short = "Queue 'x' delayed commands at 'y' ms intervals"
description = """
Delay the command following the colon 'x' times at intervals of `y` ms by placing it into a buffer.
The buffer can hold up to 16 commands. If the buffer is full, additional commands
will be discarded.
"""
["DEL.R"]
prototype = "DEL.R x y: ..."
short = "Trigger '1' command immediately, and queue 'x' minus '1' delayed commands at 'y' ms intervals"
description = """
Delay the command following the colon once immediately, and 'x' minus '1' times at intervals of `y` ms by placing it into a buffer.
The buffer can hold up to 16 commands. If the buffer is full, additional commands
will be discarded.
"""
2 changes: 2 additions & 0 deletions src/match_token.rl
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,8 @@
# delay
"PROB" => { MATCH_MOD(E_MOD_PROB); };
"DEL" => { MATCH_MOD(E_MOD_DEL); };
"DEL.X" => { MATCH_MOD(E_MOD_DEL_X); };
"DEL.R" => { MATCH_MOD(E_MOD_DEL_R); };

# matrixarchate
"MA.SELECT" => { MATCH_OP(E_OP_MA_SELECT); };
Expand Down
101 changes: 94 additions & 7 deletions src/ops/delay.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,58 @@
#include "teletype.h"
#include "teletype_io.h"

static void delay_common_add(scene_state_t *ss, exec_state_t *es, int16_t i,
int16_t delay_time,
const tele_command_t *post_command);

static void mod_DEL_func(scene_state_t *ss, exec_state_t *es,
command_state_t *cs,
const tele_command_t *post_command);

static void op_DEL_CLR_get(const void *data, scene_state_t *ss,
exec_state_t *es, command_state_t *cs);

static void mod_DEL_X_func(scene_state_t *ss, exec_state_t *es,
command_state_t *cs,
const tele_command_t *post_command);

static void mod_DEL_R_func(scene_state_t *ss, exec_state_t *es,
command_state_t *cs,
const tele_command_t *post_command);

const tele_mod_t mod_DEL = MAKE_MOD(DEL, mod_DEL_func, 1);
const tele_op_t op_DEL_CLR = MAKE_GET_OP(DEL.CLR, op_DEL_CLR_get, 0, false);
const tele_mod_t mod_DEL_X = MAKE_MOD(DEL.X, mod_DEL_X_func, 2);
const tele_mod_t mod_DEL_R = MAKE_MOD(DEL.R, mod_DEL_R_func, 2);

// common code to queue a delay shared between all delay ops
// NOTE it is the responsibility of the callee to call tele_has_delays
static void delay_common_add(scene_state_t *ss, exec_state_t *es, int16_t i,
int16_t delay_time,
const tele_command_t *post_command) {
if (i < DELAY_SIZE) {
ss->delay.count++;
Copy link
Member

Choose a reason for hiding this comment

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

i would add a check here to make sure it doesn't exceed DELAY_SIZE - imho it's a good idea to have safety checks in helper functions.

Copy link
Contributor Author

@alpha-cactus alpha-cactus Oct 12, 2018

Choose a reason for hiding this comment

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

ah good catch I can add that in. on the same topic do you think it would be good to move tele_has_delays into the helper function as well? the downside is it would get called excess times times by DEL.X, but the change would make the helper function slightly more robust for other future usage.

Copy link
Member

Choose a reason for hiding this comment

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

i'd leave it out - the function has a well defined and logical contract (it adds a delay), tele_has_delays is outside of that (and only has meaning to where this function is called from).

ss->delay.time[i] = delay_time;
ss->delay.origin_script[i] = es_variables(es)->script_number;
ss->delay.origin_i[i] = es_variables(es)->i;
copy_command(&ss->delay.commands[i], post_command);
}
}

static void mod_DEL_func(scene_state_t *ss, exec_state_t *es,
command_state_t *cs,
const tele_command_t *post_command) {
int16_t i = 0;
int16_t a = cs_pop(cs);
int16_t delay_time = cs_pop(cs);

if (a < 1) a = 1;
if (delay_time < 1) delay_time = 1;

// 0 is the magic number for an empty slot.
// Be careful not to set delay.time[i] to 0 before calling this function.
while (ss->delay.time[i] != 0 && i != DELAY_SIZE) i++;

if (i < DELAY_SIZE) {
ss->delay.count++;
ss->delay.time[i] = a;
ss->delay.origin_script[i] = es_variables(es)->script_number;
ss->delay.origin_i[i] = es_variables(es)->i;
copy_command(&ss->delay.commands[i], post_command);
delay_common_add(ss, es, i, delay_time, post_command);
tele_has_delays(ss->delay.count > 0);
}
}
Expand All @@ -42,3 +65,67 @@ static void op_DEL_CLR_get(const void *NOTUSED(data), scene_state_t *ss,
command_state_t *NOTUSED(cs)) {
clear_delays(ss);
}

static void mod_DEL_X_func(scene_state_t *ss, exec_state_t *es,
command_state_t *cs,
const tele_command_t *post_command) {
int16_t i = 0;
int16_t num_delays = cs_pop(cs);
int16_t delay_time = cs_pop(cs);
int16_t delay_time_next;

if (delay_time < 1) delay_time = 1; // minimum delay time = 1ms

delay_time_next = delay_time; // set first delay time to delay time

// 0 is the magic number for an empty slot.
// Be careful not to set delay.time[i] to 0 before calling this function.
while (ss->delay.time[i] != 0 && i != DELAY_SIZE) i++;

while (i < DELAY_SIZE && num_delays > 0) {
// queue the delay
delay_common_add(ss, es, i, delay_time_next, post_command);

// increment delay time for next delay
// normalise incremented value to stop negative wrap from increment
delay_time_next += delay_time;
delay_time_next = normalise_value(1, 32767, 1, delay_time_next);

num_delays--;
i++;
Copy link
Member

Choose a reason for hiding this comment

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

sorry, should've looked into the code closer the 1st time.. there is a problem with this spot here. for multiple delays it will find the 1st available slot but for any consecutive delays it will use the next delay without checking if it's free or not. you have to check that for each delay added.

this: while (ss->delay.time[i] != 0 && i != DELAY_SIZE) i++; can be moved to delay_common_add and i parameter can be removed, so the function's responsibility will be finding the 1st available spot and adding a delay there. it could return a bool indicating if it was added, so if false is returned this loop can assume no more spots are available and exit early.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wow another good catch. elegant solution too. I'll work on getting that together tonight.

}

tele_has_delays(ss->delay.count > 0);
}

static void mod_DEL_R_func(scene_state_t *ss, exec_state_t *es,
command_state_t *cs,
const tele_command_t *post_command) {
int16_t i = 0;
int16_t num_delays = cs_pop(cs); // number of chained delays
int16_t delay_time = cs_pop(cs); // delay time
int16_t delay_time_next; // incremented delay time

if (delay_time < 1) delay_time = 1; // minimum delay time = 1ms

delay_time_next = 1; // set first delay time to 1ms to trigger immediately

// 0 is the magic number for an empty slot.
// Be careful not to set delay.time[i] to 0 before calling this function.
while (ss->delay.time[i] != 0 && i != DELAY_SIZE) i++;

while (i < DELAY_SIZE && num_delays > 0) {
// queue the delay
delay_common_add(ss, es, i, delay_time_next, post_command);

// increment delay time for next delay
// normalise incremented value to stop negative wrap from increment
delay_time_next += delay_time;
delay_time_next = normalise_value(1, 32767, 1, delay_time_next);

num_delays--;
i++;
}

tele_has_delays(ss->delay.count > 0);
}
2 changes: 2 additions & 0 deletions src/ops/delay.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,7 @@

extern const tele_mod_t mod_DEL;
extern const tele_op_t op_DEL_CLR;
extern const tele_mod_t mod_DEL_X;
extern const tele_mod_t mod_DEL_R;

#endif
2 changes: 1 addition & 1 deletion src/ops/op.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ const tele_mod_t *tele_mods[E_MOD__LENGTH] = {
&mod_OTHER, &mod_PROB,

// delay
&mod_DEL,
&mod_DEL, &mod_DEL_X, &mod_DEL_R,

// stack
&mod_S
Expand Down
2 changes: 2 additions & 0 deletions src/ops/op_enum.h
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,8 @@ typedef enum {
E_MOD_OTHER,
E_MOD_PROB,
E_MOD_DEL,
E_MOD_DEL_X,
E_MOD_DEL_R,
E_MOD_S,
E_MOD__LENGTH,
} tele_mod_idx_t;
Expand Down
2 changes: 1 addition & 1 deletion src/state.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#define Q_LENGTH 64
#define TR_COUNT 4
#define TRIGGER_INPUTS 8
#define DELAY_SIZE 8
#define DELAY_SIZE 16
#define STACK_OP_SIZE 16
#define PATTERN_COUNT 4
#define PATTERN_LENGTH 64
Expand Down