jmamma / MCL

MCL firmware for the MegaCommand MIDI Controller.
BSD 2-Clause "Simplified" License
53 stars 9 forks source link

PageSelectPage wip #70

Closed yatli closed 4 years ago

jmamma commented 4 years ago

@yatli the last commit implements what I had in mind for pageselect changes.

Split the 16 triggers in to 4 groups

Trigs [0 - 3]

Trigs [4 - 7 ]

Trigs [8 - 13]

Trigs [14 - 15]

--

I've started disabling Encoder[1-4] buttons for exiting to GridPage. Shift 2 now becomes page select in all relevant pages.

Care needs to be taken when switching pages whether to disable then re-enable the trigger interface.

I need to work out the details here. It might not be worth disabling the Trig interface if it's to be re-enabled immediately on the next page load. If the trig interface is to be disabled then the Global Settings may need to be retransmitted to make sure the Audio routing is consistent across the two globals if a routing change has occurred. Some juggling needs to be performed here as transmitting globals can cause brief sequencer dropout.

yatli commented 4 years ago

I've implemented quick select in category. Use encoder buttons to jump within the cat. The category name is now displayed and fits 1602 LCD.

jmamma commented 4 years ago

I've implemented quick select in category. Use encoder buttons to jump within the cat. The category name is now displayed and fits 1602 LCD.

Cool i'll check this out later tonight.

--

I was thinking that Button 1 should setPage(&grid_page)

jmamma commented 4 years ago

Probably should draw_trigs() on PageSelect page to give visual indication that that the trig interfaces can be used.

jmamma commented 4 years ago

@yatli

I think we should change encoder[0] range to category size. And encoder[1] range to number of categories.

Can we change encoder behaviour from relative to absolute. I don't see the need here unless I missed something.

yatli commented 4 years ago

Can we change encoder behaviour from relative to absolute. I don't see the need here unless I missed something.

The pages are not numbered consecutively.

yatli commented 4 years ago

Also, for category, the cat id is first determined with the page index of the current page (we only know the page number, not index), so that's also relative.

yatli commented 4 years ago

@jmamma can you revert the display code? It was intended for the legacy displays. I'll be working on a new visual layout tonight for OLED.

jmamma commented 4 years ago

Also, for category, the cat id is first determined with the page index of the current page (we only know the page number, not index), so that's also relative.

Right.

Can we still achieve ->

I think we should change encoder[0] range to category size.
And encoder[1] range to number of categories.
jmamma commented 4 years ago

The original HD44780 display code is still there. Just change the OLED code.

184 void PageSelectPage::display() {
185 #ifdef OLED_DISPLAY
186   oled_display.clearDisplay();
187   char str[16];
188   oled_display.setCursor(0,0);
189   oled_display.print("Page Select:");
190   get_category_name(page_select, str);
191   oled_display.print(str);
192 
193   oled_display.setCursor(0,15);
194   get_page(page_select, str);
195   oled_display.print(str);
196   oled_display.display();
197 
198 #else
199   GUI.setLine(GUI.LINE1);
200   char str[16];
201   GUI.put_string_at_fill(0, "Page Select:");
202   get_category_name(page_select, str);
203   GUI.put_string_at(12, str);
204 
205   GUI.setLine(GUI.LINE2);
206   get_page(page_select, str);
207   GUI.put_string_at_fill(0, str);
208 #endif
209 }
210 
yatli commented 4 years ago

Oh okay, overlooked that one. :)

yatli commented 4 years ago

@jmamma the concept art:

image

jmamma commented 4 years ago

Cool!

I was debating whether or not Popup should be used.

Need to see if it looks too similar to Save/Chain pages.

yatli commented 4 years ago

If using a popup, there's no space for an icon. Thoughts?

jmamma commented 4 years ago

If using a popup, there's no space for an icon. Thoughts?

Ah yeah.

Perhaps we go for blank page layout, and try and put the icons in.

yatli commented 4 years ago

Second try:

image

Better horizontal space utilization. More futuristic. :)

jmamma commented 4 years ago

beauty.

yatli commented 4 years ago

I'll cleanup and implement this.

yatli commented 4 years ago

@jmamma it's done.

jmamma commented 4 years ago

Cool. ill test now.

We need to decide new button layout for FIle Browser and Menu.

Eliminate the use of Encoder buttons would be ideal.

Save: NO/BACK. Write YES/FORWARD ?

jmamma commented 4 years ago

@yatli looks really good. nice job

yatli commented 4 years ago

We need to decide new button layout for FIle Browser and Menu. Eliminate the use of Encoder buttons would be ideal.

Yeah. Let's try and unify the key assignments. The functionalities (and current key):

Also we should consider:

jmamma commented 4 years ago

Select entry (Enc push) -> I think we should eliminate encoder button completely where possible.

yatli commented 4 years ago

We probably need some kind of table to help designing the layout. Excel?

jmamma commented 4 years ago

I was hoping there was some online tool for this exact purpose.

jmamma commented 4 years ago

8011a07

implements my above suggestion.

There is a complication in how you access the SYSTEM MENU, and the Save / Chain pages.

Save/Write pages are opened on BUTTON_RELEASED

System Menu is exited on BUTTON_PRESSED.

When you Exit System menu you immediately trigger a BUTTON RELEASE in the GridPage and open the SAVE page.

yatli commented 4 years ago

Let's try this:

https://1drv.ms/x/s!Aql6jSQH9uyZhPpDVrfyjYOCSa56sw

jmamma commented 4 years ago

@yatli We're going to need a way of changing tracks from the MC sequencer pages.

Because the Trig interface is active in the PageSelect page it won't be possible to switch tracks unless exiting back to Grid.

I'm thinking [ Shift 2] opens up a track menu with

Thoughts?

yatli commented 4 years ago

Yeah that's what I thought about. One comment is about Clear current track. If it's moved from WRITE to the menu, it'll be a bit harder to invoke, which slows down real time dubbing/clearing workflow.

A workaround is to have the menu "sticky" -- if one selects clear track, the selection is not reset to the first entry when re-entering the menu.

S2 can then be thought of as a reconfigurable key -- by default it does what the user set last time. If last action is clear track, then it stays clear track, until reconfigured with enc2

yatli commented 4 years ago

Current progress:

image

S1 has a very consistent press-and-hold sense. S2 is mostly ex-menu, or backspace in text edit (could also be a menu, include backspace/clear/case).

The grid page S2+Enc push was "load and edit with seq page", currently commented out. Possible to add an option to re-enable this?

jmamma commented 4 years ago

Added: wavd_icon.png loudness_icon.png

improved: lfo_icon.png mixer_icon.png (matches display of mixer page)

jmamma commented 4 years ago

Nice work @yatli !

jmamma commented 4 years ago
169   if (EVENT_RELEASED(event, Buttons.BUTTON3)) {
170     encoders[0] = &seq_param1;
171     encoders[1] = &seq_param2;

Because we do this, the underlying sequencer page draw_knob() values change as encoders 1 and 2 are rotated when show_seq_menu == true.

If I recall, my solution for GridPage was to hardcode the encoder pointers in the sequencer pages member functions.

Unless there is a better way?

jmamma commented 4 years ago

Track Select, offset by 1 so that the number are 1->16.

Also, track select beyond 5 doesn't appear to work

jmamma commented 4 years ago
jmamma commented 4 years ago

Page Select Icon's are so beautiful 😂

yatli commented 4 years ago

@jmamma I'm burning out. It's still a bit messy, could you help to clean it up a bit?

jmamma commented 4 years ago

Will look at it tomorrow. Need to eat dinner and have a few hours of rest.

Thanks for your effort!

yatli commented 4 years ago

Incorrect behaviors:

yatli commented 4 years ago

When operating the menu, it's easy to trigger S2 + ENC3, which is currently change all track length. I'm changing it to WRITE + ENC3.

yatli commented 4 years ago

Cleaning up code, powered by zen inertia. I've no idea what I'm writing but it fixes problems!

yatli commented 4 years ago

It's taking the shape now. I'll leave the stubs (copy, paste etc.) to you.

yatli commented 4 years ago

ALL DONE

yatli commented 4 years ago

I've compiled the current key assignment table: image

It's in the second sheet in the excel link.

jmamma commented 4 years ago

@yatli

I'm wondering if SoundBrowser should be moved from PageSelect in to TrackMenu. Kind of makes more sense as it is related to the current track.

Thoughts?

jmamma commented 4 years ago

TrackMenu vs GridSlotMenu

Different behaviour relating to clear / copy / paste

In grid slot Menu.

Clear: [ -- , YES ]
Copy: [ -- , YES ]
Paste: [ -- , YES ]

When you close the menu options revert back to --

Where as Track Menu

Clear: [ TRK , ALL ]
Copy: [ TRK, ALL ]
Paste: [ TRK, ALL ]

The side effect of this is that when you open up the track menu, it may already be positioned over Clear, and releasing the button may clear your track or all your tracks unintentionally.

Should we use the GridSlotMenu method? Or should we use the TrackMenu method, but add a confirmation dialog box?

The other option is to reset menu item position to "Track Select", but this feels weird and slows down workflow imo.

yatli commented 4 years ago

Should we use the GridSlotMenu method? Or should we use the TrackMenu method, but add a confirmation dialog box?

I prefer GridSlotMenu method. As an extra safety guard, we can implement undo in another PR.

I'm wondering if SoundBrowser should be moved from PageSelect in to TrackMenu.

Hmm. Considering the workflow, when editing the patch MC automatically exits to the GRID. If the sound browser is activated via TrackMenu, then it's going to either pull an untouched MD patch, or, go from GRID -> Seq -> TrackMenu

Put it in the GridMenu maybe?

jmamma commented 4 years ago

@yatli copy/paste is working for MDTracks*

I decided to re-use MCLClipboard. Means the copied tracks are not stored in RAM but on the SDCard.

give it a test when you have some time.

jmamma commented 4 years ago

Left Right shifting is working, but parameter locks are not being copied correctly.

Anything obvious here:


519 void MDSeqTrack::rotate_right() {
520 
521   ROTATE_RIGHT(lock_mask, length);
522   ROTATE_RIGHT(pattern_mask, length);
523   ROTATE_RIGHT(oneshot_mask, length);
524 
525   int8_t new_pos = 0;
526 
527   MDSeqTrackData temp_data;
528 
529   memcpy(&temp_data, this, sizeof(MDSeqTrackData));
530 
531   for (uint8_t n = 0; n < length; n++) {
532      if (n == length - 1) { new_pos = 0; }
533      else { new_pos = n + 1; }
534      memcpy(&locks[0][new_pos], &(temp_data.locks[0][n]), NUM_MD_LOCKS);
535      conditional[new_pos] = temp_data.conditional[n];
536      timing[new_pos] = temp_data.timing[n];
537   }
538 
539 }

@yatli ?

jmamma commented 4 years ago

ah memcpy won't work. because of the array order.