this seems like a good approach to me.

oh yes, this is/was a bad idea. screen redraw should happen in the event handler-- thank you for catching this.

2 Likes

Done.

Managed to remove screen_dirty as a global variable too as the code became much more logical with the screen updating in handler_ScreenRefresh.

5 Likes

good idea and nice use of c99 !

1 Like

indeed, timer callbacks should only ever post events for the main loop. good catch

1 Like

@zebra and @tehn, one more question about event posting…

At the moment it is done with a static event_t, e.g.:

static void refreshTimer_callback(void* o) {
    static event_t e;
    e.type = kEventScreenRefresh;
    e.data = 0;
    event_post(&e);
}

What’s the reasoning behind making it static?

There is a section of event_post when the data structures are updated where interrupts are masked, but otherwise (hypothetically) if refreshTimer_callback is called twice concurrently (i.e. in an interrupt) and if the .data member was different (see tele_ii), couldn’t the second call overwrite the data from the first before it was added to the event queue?

Why not just declare the variable locally on then stack and pass it in?

static void refreshTimer_callback(void* o) {
    event_t e = {.type = kEventScreenRefresh, .data = 0 };
    event_post(&e);
}
1 Like

Here we go then… beta 2 (zip file in the first post).

Changes since beta 1.

  • (breaking) No more II op. Don’t do II WW.PRESET 1, instead just do WW.PRESET 1
  • (breaking) No more UNMUTE, instead use MUTE x to get the status of the mute on input x, and MUTE x y to set the mute on input x (y = 0 to disable mute, y = 1 to enable mute).
  • Rewritten key bindings.
  • 16 history entries in live mode instead of 6.
  • Global cut and paste between live, edit, and preset write modes.
  • <esc> key handling changed. No more getting stuck between preset write and preset read mode (this always happened to me anyway).
  • Limited script recursion (max recursion depth is 8) including self recursion.
  • No more leading spaces before : and ;

Please look at the new key bindings, the major changes are no more ~ key to enter pattern mode. Mode changes are only done using <tab>. I will admit it took me a bit of time to get used to that and I’m not sure if it’s the right choice or not. But please, before you raise pitchforks about it give yourself a bit of time to get used to it.

The help menu is now on <print screen>.

F1 though F10 trigger scripts, and you can use an alt modifier to jump to edit that script.

I’ve switched to using alt and win to describe the keyboard shortcuts rather than using the technically more correct meta as that’s what printed on the keyboard.

Some keybindings may be missing. Toggling mutes from the keyboard definitely is. Suggestions for what it should be?

(There was a typo in the keybinding docs, F9 is the metro script, F10 is the init script, docs updated now.)


GitHub tells me that I’ve changed 59 files and made 3,593 additions and 2,890 deletions since beta 1. :dizzy_face:

@tehn I’d like to put a PR in for this, even if I’ve still got some more stuff I’d like to get done for the release. Cool?

10 Likes

yes to a PR! amazing!!

1 Like

there is no reason for that to be static. IMO it’s a mistake.

as it happens, there’s no race condition because all the soft-timer callbacks are always called from a single timer interrupt, and avr32 interrupts aren’t nested unless the handler explicitly re-enables the IRQ.

i see that i made the same mistake in the bees encoder-polling handler. (similarly without consequences.) maybe it was copied from there.

as to the reason for this (anti-)pattern, it’s something i used to see a lot in embedded C code in days past, on much more limited systems. for an extremely hot ISR on (say) 8051, it was actually a good idea to minimize stack allocation. stupid habit now.

3 Likes

So by avoiding using the stack in an ISR we avoid having to push and pop the stack address register on the context switch?

I found an interesting article on the ARM website about interrupt latency, it’s a bit ‘wow ARM CPUs are amazing’, but an interesting read nonetheless. I also think ARM Cortex chips have 2 stack pointer registers to make context switches cheaper. I’m guessing many modern CPUs use similar tricks.

does this update preserve numpad functionality? i wonder if i’m the only person that uses it…

Does this release takes over @tehn Teletype V2 proposal? I understand they are 2 different firmware but does this means that this version kind of defacto supersedes that proposal?

mm… TBH, i am not totally sure now. it gets a bit confusing.

had a quick look at the avr32 architecture manual again; AVR32B (which teletype uses) has low-interrupt-latency features; from section 1.3.2:

implements dedicated registers to hold the status register and return
address for interrupts, exceptions and supervisor calls. This information does not need to be written to the stack, and latency is therefore reduced. Additionally, AVR32B allows hardware shadowing of the registers in the register file. The INT0 to INT3 contexts may have dedicated versions of the registers in the register file, allowing the interrupt routine to start executing immediately.

aleph uses UC3A, which doesn’t have those features, and saves SR, etc. on the system stack, the old-fashioned way. (there is still a dedicated shadow register for SP, only accessible by privileged modes… this is the confusing bit for me.) if you’re going to get a stack overflow, it is likely to be in an ISR, so it’s not totally crazy to avoid stack allocations in them. i doubt it will impact performance sigificantly, at least on UC3B, but it would be interesting to try to time it (by scoping a GPIO interrupt, say) and to look at the objdumps…

bottom line is i think it’s safer as you say to use local stack variables and avoid the possibility of race conditions; the hypothetical ISR latency improvement is probably not worth it.

(sorry for this minor derailment of the thread, we could move the topic somewhere else)

It does (but I don’t have the ability to test it), the key bindings docs should be exhaustive, they were made by going through the code. Do you use it with a standalone numpad?

This will be the next official firmware. The V2 features can still be implemented on top of this, though I think it would probably be wise to revisit them (e.g. was the primary use of timelines a way to allow longer scripts, and if so, do the sub commands meet the same aim?).

From what I can tell the 2 shadow SP registers, called MSP and PSP on ARM chips (main stack pointer and privileged stack pointer), along with the very limited memory protection available enable you to have supervisor code that can’t be interfered with by unprivileged code (useful for say, FreeRTOS).

I agree too. I’ll definitely change the ones in the teletype code base, I may change the ones in init_teletype.c too.

3 Likes

How about using with 1 - 8 to toggle mute/unmute for the trigger inputs? Haven’t done the update yet cause I did not succeed in saving my scenes to USB by now but I will definitely miss this feature.

Anyway, thanks for doing all the work - I am really looking forward to a lot of the new features!

Just a quick heads up, there is a bug in beta 2, where preset saving only saves script 1 to flash. Fix incoming.


Beta 3 available on the first post with fix.


FYI I’ve noticed another bug, the bar at the top of the screen in live mode doesn’t always display. I’ve fixed it already, and that fix will be in the next beta.

I’m seeing the same screen artifacts that @Leverkusen reported when I’m switching modes. It will happen for a few seconds and then correct itself.

I’m also experiencing a save/load or scene change bug (not sure which). I have the default TT scripts on a USB stick. I just loaded them back onto my TT now that this version was more stable for me. If I load Triangle Mountain and then switch to 4Track, it keeps the tracker data from T.Mountain and doesn’t load the 4 lanes saved with 4Track.

1 Like

I think I might know what’s causing that. Does it end up only loading the data from pattern 0, but patterns 1, 2 & 3 stay the same? You might find that you get the same thing with both USB loading/saving and preset loading/saving.

I’ll get it fixed tomorrow most likely. Then I’ll release a new beta with the fix and the new i2c code included too.

3 Likes

Yep! Pattern 0 loads correctly, but 1, 2, and 3 are always blank.

1 Like

Beta 4 posted in the first post.

Changes since beta 3.

  • Switch the metro script (and delayed commands) to running at the next available opportunity, rather than immediately. See this post and onwards for details as to why. The downside is that the metro clock may be a bit more jittery depending on system load. The upside is that i2c / II crashes are significantly reduced if not eliminated entirely… (famous last words…)
    Enormous thanks to @trickyflemming for all the testing he did as well as @tehn for coming up with the solution and @Leverkusen for his testing too.
  • Fix the pattern saving bug that @trickyflemming reported. Annoyingly the cause was the same as the script saving bug that necessitated beta 3.
  • Make sure that the activity bar in live mode always displays.

There are also some underlying changes to how II messages to the original trilogy modules are calculated, I’ve tried my hardest to check the values are correct, but due to my development setup it’s not that easy to check on actual hardware. I would appreciate feedback to make sure the II messages still work (and remember you don’t need to use the II op anymore).

Assuming there aren’t any major bugs I will get a PR in for this code this weekend… and then onto sorting out the USB memory stick issues.

6 Likes

Sam,

I appreciate your work on this very much and am currently trying out the new firmware - thanks a lot for all the afford you are dedicating to this!

There is just one thing I am not sure about. That is the new triple switch to change between tracker view, live view and edit view. I used to switch from edit view to live view back and forth to just change global values as the metro speed, single trigger pulse widths or the es.mode without looking on the screen - just a short press, typing something like M 100 and switching back to work on the script. Now I am constantly overwriting the pattern when I automatically do so and have to look on the screen where I am which is a bit cumbersome.

I understand live and edit modes as two sides of teletype functionality where it makes sense to switch between but the tracker view is somehow a different thing plus triple switching seems counter intuitive to me as you cannot do it blind.

Is there a special reason for the new method? Would you consider the old way coming back some day?

Thanks again for the great support,

Sven

btw: at a first glance i2c seems good with reading cycles and triggering non existing trigger.outs with a metro script and trigger input…:heart_eyes:

2 Likes