tttapa / Control-Surface

Arduino library for creating MIDI controllers and other MIDI devices.
GNU General Public License v3.0
1.23k stars 137 forks source link

Is it possible to send relative PB changes in MCU using a rotary encoder? #328

Open mrmike12333 opened 3 years ago

mrmike12333 commented 3 years ago

Hi!

I'm trying something experimental and perhaps going about this the completely wrong way. I want to change the volume of tracks (in Logic Pro X) through a rotary encoder in the MCU bank. I've got an almost working version, but it only sends absolute values through pitch bend - i.e. min or max values.

I've implemented this through a 'Bankable' rotary encoder that sends Pitch bends and want to know what is the best method of handling relative changes.

BEGIN_CS_NAMESPACE

namespace Bankable {
    class CustomRotaryEncoder
        : public MIDIRotaryEncoder<SingleAddress, PitchBendSender<14>> 
        {
          public:      
            CustomRotaryEncoder(
                OutputBankConfig<> config, const EncoderPinList &pins, const MIDIChannelCN &address, 
                int16_t multiplier = 1, uint8_t pulsesPerStep = 4) 
                : MIDIRotaryEncoder({config, address}, pins, multiplier, pulsesPerStep, {}) {}          
        };
}

END_CS_NAMESPACE

Please let me know your thoughts on this, as I'm happy to attempt a different approach if it's simpler.

tttapa commented 3 years ago

AFAIK, there's no standard way to send relative values using Pitch Bend messages. The MCU protocol uses Control Change messages for relative values, and so do all other protocols and DAWs I've come across. If you want to use an encoder to control an absolute Pitch Bend controller like the MCU volume faders, you can use the PBAbsoluteEncoder class. Do keep in mind that this keeps the absolute position locally, and it sends this absolute position to the DAW. This means that it has the same issues as an absolute potentiometer that's shared across banks. There's no bankable variant on the master branch, but Bankable::PBAbsoluteEncoder is available on the new-input development branch.

You could make PBAbsoluteEncoder smarter by listening to the position feedback from the DAW. If you want to go that route, let me know and I'll provide some more information.

If you find a way to allow your DAW to receive and interpret “relative” Pitch Bend messages, you can easily add your own MIDI sender to support it on the Arduino with Control Surface as well, you can get inspiration from the RelativeCCSender class.

mrmike12333 commented 3 years ago

I've had a try with the new-input brach with the Bankable::PBAbsoluteEncoder and they seem to do pretty much what I'm looking for! The only exception, is on a new bank it stores the value locally, so when moving through banks and on the initial potentiometer change it will set the fader to 0.
I'd be interested to know how to initialise the banks by listening to the DAW positions.

On a related note, I've been having issues using multiple Encoders. The issue in question comes from the board 'locking' or 'freezing' after turning one of the rotary encoders. The serial monitor does not show anything coming from the board and the only fix is a hard reset.

After a lot of debugging I can rule out that it's not the individual encoders, board or pins. By using a simple script, such as the ones found here I can push these to my board and get the expected behaviour from the rotary encoders.

Here is my script for reference. I'm using an Arduino MKR ZERO.

#include <Encoder.h>
#include <Control_Surface.h>

//USBDebugMIDI_Interface midi(115200);
USBMIDI_Interface midi;

const uint8_t NumBanks = 2 ;
const uint8_t NumFaders = 4;

Bank<NumBanks> bank(NumFaders);

Bankable::NoteButton soloMutes[] = {
  {bank, 2, MCU::SOLO_1},
  {bank, 5, MCU::SOLO_2}
};

const int16_t Mult = 1;
const uint8_t rot = 4;

// DT, CLK
Bankable::PBAbsoluteEncoder<NumBanks> volumeTrimEncoders[] = {
  {bank, {2, 3}, MCU::VOLUME_1, Mult, rot},
  {bank, {4, 5}, MCU::VOLUME_2, Mult, rot},
};

void setup() {
  RelativeCCSender::setMode(MACKIE_CONTROL_RELATIVE);
  Control_Surface.begin();
}

void loop() {
  Control_Surface.loop();
}

If I remove the PBAbsoluteEncoders then everything works perfectly, however I'm now getting a strange issue where the code compiles and finishes uploading then 'closes the port' so it no longer appears on the IDE and it spits out an error (see below). I should also note that this occurs with nothing connected to the Arduino board and this behaviour does not occur if the PBAbsoluteEncoders are commented out.

ketch uses 23440 bytes (8%) of program storage space. Maximum is 262144 bytes.
Global variables use 3740 bytes (11%) of dynamic memory, leaving 29028 bytes for local variables. Maximum is 32768 bytes.
Atmel SMART device 0x10010005 found
Device       : ATSAMD21G18A
Chip ID      : 10010005
Version      : v2.0 [Arduino:XYZ] Dec 20 2016 15:36:46
Address      : 8192
Pages        : 3968
Page Size    : 64 bytes
Total Size   : 248KB
Planes       : 1
Lock Regions : 16
Locked       : none
Security     : false
Boot Flash   : true
BOD          : false
BOR          : false
Arduino      : FAST_CHIP_ERASE
Arduino      : FAST_MULTI_PAGE_WRITE
Arduino      : CAN_CHECKSUM_MEMORY_BUFFER
Erase flash
done in 0.823 seconds

Write 23440 bytes to flash (367 pages)
[==============================] 100% (367/367 pages)
done in 0.158 seconds

Verify 23440 bytes of flash with checksum.
Verify successful
done in 0.020 seconds
CPU reset.
processing.app.SerialException: Error opening serial port '/dev/cu.usbmodem14103'.
    at processing.app.Serial.<init>(Serial.java:152)
    at processing.app.Serial.<init>(Serial.java:82)
    at processing.app.SerialMonitor$2.<init>(SerialMonitor.java:132)
    at processing.app.SerialMonitor.open(SerialMonitor.java:132)
    at processing.app.AbstractMonitor.resume(AbstractMonitor.java:132)
    at processing.app.Editor.resumeOrCloseSerialMonitor(Editor.java:2120)
    at processing.app.Editor.access$1300(Editor.java:117)
    at processing.app.Editor$UploadHandler.run(Editor.java:2089)
    at java.lang.Thread.run(Thread.java:748)
Caused by: jssc.SerialPortException: Port name - /dev/cu.usbmodem14103; Method name - openPort(); Exception type - Port not found.
    at jssc.SerialPort.openPort(SerialPort.java:167)
    at processing.app.Serial.<init>(Serial.java:141)
    ... 8 more
Error opening serial port '/dev/cu.usbmodem14103'.
tttapa commented 3 years ago

That's very strange. Could you try if the problem manifests itself when just using multiple encoders using the https://github.com/PaulStoffregen/Encoder library directly? Using this example, for instance: https://github.com/PaulStoffregen/Encoder/blob/master/examples/TwoKnobs/TwoKnobs.pde.

I'll see if I can reproduce the problem on other boards, but I don't own an Arduino MKR ZERO myself.

mrmike12333 commented 3 years ago

I've tried the exact TwoKnobs code you've sent and it works perfectly, but going back to the script I sent earlier has the same issues. Unfortunately I don't have another board to test this on that has USB functionality.

tttapa commented 3 years ago

Control Surface uses a modified version of the PJRC Encoder library. The only relevant difference is that the modified version uses the digitalPinToInterrupt macro instead of a huge switch statement. (I might be missing another important difference, of course).

I've tried it on AVR, Teensy and ESP32, and it seems to work fine on these boards. Looking at the SAMD interrupt code, there used to be a problem with the digitalPinToInterrupt macro, but it should have been fixed a long time ago. What version of the Arduino SAMD Core are you using? (Go to Tools > Board > Boards Manager in the Arduino IDE to find the exact version.)

Are you using the same pins as the TwoKnobs example? Have you tried the PBAbsoluteEncoder sketch you posted on these pins or different ones?

This issue might be of interest: https://github.com/arduino/ArduinoCore-samd/issues/537

The Encoder library also mentions this:

// Arduino Zero - TODO: interrupts do not seem to work
//                      please help, contribute a fix!

Could you test the following sketch for each of the 4 pins of your four encoders? Upload the sketch, open the serial monitor, and turn the encoder. You should see the the count increasing when the encoder moves.

const uint8_t interruptpin = 7;
static volatile unsigned long count = 0;

void setup() {
  Serial.begin(115200);
  pinMode(interruptpin, INPUT_PULLUP);
  delay(10);
  attachInterrupt(digitalPinToInterrupt(interruptpin), +[]() {
    ++count;
  }, CHANGE);
}

void loop() {
  static unsigned long prevCount = 0;
  noInterrupts();
  unsigned long tempCount = count;
  interrupts();
  if (prevCount != tempCount) {
    Serial.println(tempCount);
    prevCount = tempCount;
  }
}
mrmike12333 commented 3 years ago

What version of the Arduino SAMD Core are you using?

I'm using version 1.8.9 of Arduino SAMD Boards

Are you using the same pins as the TwoKnobs example? Have you tried the PBAbsoluteEncoder sketch you posted on these pins or different ones?

I've tried a variety of different pins, however I've still had no luck. Currently I have it configured to pins 1/2 & 4/5 and this works on the TwoKnobs example but does not work with the script I posted.

This issue might be of interest: arduino/ArduinoCore-samd#537

I've tried my script with the above pins (2,3) & (8,9) and the behaviour does not seem to change, although the port no longer disappears.

Could you test the following sketch for each of the 4 pins of your four encoders?

Interestingly, this seems to work fine on some pins, but not on others. Pins 0, 1, 4, 5, 6, 7, 8, 9 work but the others have no serial output. However, on seeing this pinout picture it makes a lot more sense, as these are the interrupt pins on the MKR ZERO.

Further to this, if I test this on the PBAbsoluteEncoders and use a single encoder on pins (0,1) - which I know works from that script - everything will work absolutely fine until I turn the encoder and serial communication ends! I'm testing using the below script. I'm basically testing to see when "Connected to Serial Bus" stops printing.

Finally, this behaviour is reproducible on a button press and does not require an encoder to replicate. This would suggest to me that the interrupt pins being used are locking the board but only when being read (I'm not too clued up on the interrupt mechanics).

#include <Encoder.h>
#include <Control_Surface.h>

USBDebugMIDI_Interface midi(115200);
//USBMIDI_Interface midi;

const uint8_t NumBanks = 2 ;
const uint8_t NumFaders = 4;

Bank<NumBanks> bank(NumFaders);

const int16_t Mult = 1;
const uint8_t rot = 4;

// encoder pins
int PinA = 0;
int PinB = 1;

// DT, CLK
Bankable::PBAbsoluteEncoder<NumBanks> volumeTrimEncoders = {
  bank, {PinA, PinB}, MCU::VOLUME_1, Mult, rot
};

void setup() {
  RelativeCCSender::setMode(MACKIE_CONTROL_RELATIVE);
  Control_Surface.begin();
}

void loop() {
  Serial.println("Connected to serial bus");
  Control_Surface.loop();
}
tttapa commented 3 years ago

I don't know why this is happening. Either something goes terribly wrong in the interrupt configuration, or for some reason the encoder interrupt context is nullptr.

I'll strip out all the unnecessary stuff and push a revised Encoder.h file that's easier to debug.

tttapa commented 3 years ago

I've pushed it to https://github.com/tttapa/Control-Surface/tree/debug-encoder-mkr-zero.

You can verify that the initialization is successful using the following sketch:

#define private public // Access Encoder's private members for debugging (don't do this in real code)

#include <Control_Surface.h>

USBDebugMIDI_Interface midi;

PBAbsoluteEncoder pbenc1 {
  {0, 1},
  CHANNEL_1,
  127,
};

PBAbsoluteEncoder pbenc2 {
  {4, 5},
  CHANNEL_2,
  127,
};

void setup() {
  Serial.begin(115200);
  while (!Serial);
  delay(100);

  Control_Surface.begin(); // Initialize Control Surface
  Serial << "Init done" << endl;

  Serial << "Encoder 1: " << hex << uintptr_t(&pbenc1.encoder.encoder) << dec << endl;
  Serial << "Encoder 2: " << hex << uintptr_t(&pbenc2.encoder.encoder) << dec << endl;

  for (size_t i = 0; i < ENCODER_ARGLIST_SIZE; ++i)
    Serial << "ISR " << i << " argument: " << hex << uintptr_t(Encoder::interruptArgs[i]) << dec << endl;

  delay(1000);
}

void loop() {
  Control_Surface.loop(); // Update the Control Surface
  delay(1000);
}

You should see something like this in the Serial Monitor:

Init done
Encoder 1: 0467
Encoder 2: 0445
ISR 0 argument: 0467
ISR 1 argument: 0467
ISR 2 argument: 00
ISR 3 argument: 00
ISR 4 argument: 0445
ISR 5 argument: 0445
ISR 6 argument: 00
ISR 7 argument: 00
...
mrmike12333 commented 3 years ago

Hi! Apologies first for taking a while to get back, I've not quite had the time to work on this recently.

Strangely enough, when I run this script I get the same error in my IDE from here. However, the board does re-connect and when the serial bus reconnects I do get the following:

22:00:18.289 -> Init done
22:00:18.289 -> Encoder 1: 20000380
22:00:18.289 -> Encoder 2: 200003bc
22:00:18.289 -> ISR 0 argument: 20000380
22:00:18.289 -> ISR 1 argument: 20000380
22:00:18.289 -> ISR 2 argument: 00
22:00:18.289 -> ISR 3 argument: 00
22:00:18.289 -> ISR 4 argument: 200003bc
22:00:18.289 -> ISR 5 argument: 200003bc
22:00:18.289 -> ISR 6 argument: 00
22:00:18.289 -> ISR 7 argument: 00
22:00:18.289 -> ISR 8 argument: 00
22:00:18.289 -> ISR 9 argument: 00
22:00:18.289 -> ISR 10 argument: 00
22:00:18.289 -> ISR 11 argument: 00
22:00:18.289 -> ISR 12 argument: 00
22:00:18.289 -> ISR 13 argument: 00
22:00:18.289 -> ISR 14 argument: 00
22:00:18.289 -> ISR 15 argument: 00
22:00:18.289 -> ISR 16 argument: 00
22:00:18.289 -> ISR 17 argument: 00
22:00:18.289 -> ISR 18 argument: 00
22:00:18.289 -> ISR 19 argument: 00
22:00:18.289 -> ISR 20 argument: 00
22:00:18.289 -> ISR 21 argument: 00
22:00:18.289 -> ISR 22 argument: 00
22:00:18.289 -> ISR 23 argument: 00
22:00:18.289 -> ISR 24 argument: 00
22:00:18.289 -> ISR 25 argument: 00
22:00:18.289 -> ISR 26 argument: 00
22:00:18.289 -> ISR 27 argument: 00
22:00:18.289 -> ISR 28 argument: 00
22:00:18.289 -> ISR 29 argument: 00
22:00:18.289 -> ISR 30 argument: 00
22:00:42.262 -> Pitch Bend          Channel: 1  Data 1: 0x7e    Data 2: 0x01    Cable: 1
22:00:43.273 -> Pitch Bend          Channel: 1  Data 1: 0x7b    Data 2: 0x04    Cable: 1
22:00:44.293 -> Pitch Bend          Channel: 1  Data 1: 0x00    Data 2: 0x00    Cable: 1
22:00:46.270 -> Pitch Bend          Channel: 1  Data 1: 0x11    Data 2: 0x0e    Cable: 1
22:00:47.283 -> Pitch Bend          Channel: 1  Data 1: 0x2f    Data 2: 0x0f    Cable: 1
22:00:48.267 -> Pitch Bend          Channel: 1  Data 1: 0x0f    Data 2: 0x10    Cable: 1
22:00:56.275 -> Pitch Bend          Channel: 2  Data 1: 0x73    Data 2: 0x0c    Cable: 1
22:00:57.271 -> Pitch Bend          Channel: 2  Data 1: 0x00    Data 2: 0x00    Cable: 1
22:01:00.282 -> Pitch Bend          Channel: 2  Data 1: 0x7a    Data 2: 0x05    Cable: 1
22:01:01.259 -> Pitch Bend          Channel: 2  Data 1: 0x2c    Data 2: 0x13    Cable: 1
22:01:02.280 -> Pitch Bend          Channel: 2  Data 1: 0x1d    Data 2: 0x22    Cable: 1
22:01:03.253 -> Pitch Bend          Channel: 2  Data 1: 0x4f    Data 2: 0x11    Cable: 1
22:01:04.278 -> Pitch Bend          Channel: 2  Data 1: 0x3a    Data 2: 0x06    Cable: 1
tttapa commented 3 years ago

That looks fine to me, all ISR arguments are initialized correctly, so if attachInterrupt works correctly (at least for the interrupt-capable pins), then the encoders should also work.

Maybe it has to do with the delays? Does it still work if you remove (some of) the delays?

mrmike12333 commented 3 years ago

Unfortunately removing the delays does not seem to change their behaviour, everything seems to be working as expected with that script.

Also, when I change the script to use the bankable encoders, I'm getting the expected behaviour in Logic Pro and neither encoder's are crashing the Arduino!

So the following script:

#define private public // Access Encoder's private members for debugging (don't do this in real code)

#include <Control_Surface.h>

USBDebugMIDI_Interface midi;
//USBMIDI_Interface midi;

const uint8_t NumBanks = 2 ;
const uint8_t NumFaders = 4;

Bank<NumBanks> bank(NumFaders);

Bankable::PBAbsoluteEncoder<NumBanks> volumeTrimEncoders[] = {
  {bank, {0, 1}, MCU::VOLUME_1},
  {bank, {4, 5}, MCU::VOLUME_2}
};

void setup() {
  Serial.begin(115200);
  while (!Serial);

  Control_Surface.begin(); // Initialize Control Surface
  Serial << "Init done" << endl;

  Serial << "Encoder 1: " << hex << uintptr_t(&volumeTrimEncoders[0].encoder.encoder) << dec << endl;
  Serial << "Encoder 2: " << hex << uintptr_t(&volumeTrimEncoders[1].encoder.encoder) << dec << endl;

  for (size_t i = 0; i < ENCODER_ARGLIST_SIZE; ++i)
    Serial << "ISR " << i << " argument: " << hex << uintptr_t(Encoder::interruptArgs[i]) << dec << endl;
}

void loop() {
  Control_Surface.loop(); // Update the Control Surface
}

produces the output below...

23:15:22.160 -> Init done
23:15:22.160 -> Encoder 1: 2000398
23:15:22.198 -> Encoder 2: 200003dc
23:15:22.198 -> ISR 0 argument: 20000398
23:15:22.198 -> ISR 1 argument: 2000039
23:15:22.236 -> ISR 2 argument: 00
23:15:22.236 -> ISR 3 argument: 00
23:15:22.236 -> ISR 4 argument: 200003dc
23:15:22.236 -> ISR 5 argument: 200003dc
23:15:22.236 -> ISR 6 argument: 00
23:15:22.236 -> ISR 7 argument: 00
23:15:22.236 -> ISR 8 argument: 00
23:15:22.236 -> ISR 9 argument: 00
23:15:22.236 -> ISR 10 argument: 00
23:15:22.236 -> ISR 11 argument: 00
23:15:22.236 -> ISR 12 argument: 00
23:15:22.236 -> ISR 13 argument: 00
23:15:22.236 -> ISR 14 argument: 00
23:15:22.236 -> ISR 15 argument: 00
23:15:22.236 -> ISR 16 argument: 00
23:15:22.236 -> ISR 17 argument: 00
23:15:22.236 -> ISR 18 argument: 00
23:15:22.236 -> ISR 19 argument: 00
23:15:22.236 -> ISR 20 argument: 00ISR 21 argument: 00
23:15:22.274 -> ISR 22 argument: 00
23:15:22.274 -> ISR 23 argument: 00
23:15:22.274 -> ISR 24 argument: 00
23:15:22.274 -> ISR 25 argument: 00
23:15:22.274 -> ISR 26 argument: 00
23:15:22.274 -> ISR 27 argument: 00
23:15:22.274 -> ISR 28 argument: 00
23:15:22.274 -> ISR 29 argument: 00
23:15:22.274 -> ISR 30 argument: 00
23:15:39.503 -> Pitch Bend          Channel: 2  Data 1: 0x01    Data 2: 0x00    Cable: 1
23:15:39.540 -> Pitch Bend          Channel: 2  Data 1: 0x02    Data 2: 0x00    Cable: 1Pitch Bend          Channel: 2  Data 1: 0x03    Data 2: 0x00    Cable: 1
23:15:39.579 -> Pitch Bend          Channel: 2  Data 1: 0x04    Data 2: 0x00    Cable: 1
23:15:39.690 -> Pitch Bend          Channel: 2  Data 1: 0x05    Data 2: 0x00    Cable: 1
23:15:40.094 -> Pitch Bend          Channel: 2  Data 1: 0x06    Data 2: 0x00    Cable: 1
23:15:40.202 -> Pitch Bend          Channel: 2  Data 1: 0x07    Data 2: 0x00    Cable: 1
23:15:40.391 -> Pitch Bend          Channel: 2  Data 1: 0x08    Data 2: 0x00    Cable: 1
23:15:40.465 -> Pitch Bend          Channel: 2  Data 1: 0x09    Data 2: 0x00    Cable: 1
23:15:41.713 -> Pitch Bend          Channel: 1  Data 1: 0x01    Data 2: 0x00    Cable: 1
23:15:41.787 -> Pitch Bend          Channel: 1  Data 1: 0x02    Data 2: 0x0 Cable: 1
23:15:41.859 -> Pitch Bend          Channel: 1  Data 1: 0x03    Data 2: 0x00    Cable: 1
23:15:41.896 -> Pitch Bend          Channel: 1  Data 1: 0x04    Data 2: 0x00    Cable: 1
23:15:41.934 -> Pitch Bend          Channel: 1  Data 1: 0x05    Data 2: 0x00    Cable: 1
23:15:42.077 -> Pitch Bend          Channel: 1  Data 1: 0x06    Data 2: 0x00    Cable: 1
23:15:42.301 -> Pitch Bend          Channel: 1  Data 1: 0x07    Data 2: 0x00    Cable: 1
23:15:42.412 -> Pitch Bend          Channel: 1  Data 1: 0x08    Data 2: 0x00    Cable: 1

On further debugging of my previous scripts, I had #include <Encoder.h> at the top of each script - removing that include fixes the issue and including the encoder header crashes on turning the encoder at pins {0,1}.

tttapa commented 3 years ago

On further debugging of my previous scripts, I had #include at the top of each script - removing that include fixes the issue and including the encoder header crashes on turning the encoder at pins {0,1}.

That makes sense, if you include Encoder.h yourself, it seems to clash with the built-in Encoder class. I've wrapped the built-in one in a namespace, so this shouldn't happen anymore.

Does removing the Encoder.h include fix all problems, or are there other things you need help with?

mrmike12333 commented 3 years ago

Does removing the Encoder.h include fix all problems

Yes, or at least it appears to, I'll keep monitoring this and will let you know if I encounter any bugs, but so far so good! Many thanks on helping me track down this issue.

You could make PBAbsoluteEncoder smarter by listening to the position feedback from the DAW. If you want to go that route, let me know and I'll provide some more information.

I am still quite interested in implementing this, let me know if you have some recommendations on what would be the best way to approach this.

tttapa commented 3 years ago

I am still quite interested in implementing this, let me know if you have some recommendations on what would be the best way to approach this.

I seem to have missed your previous message. Do you still need help with it?

yuri-evangelista commented 3 years ago

Hi, I would also be very interested in using absolute encoders for volumes by listening to the positions from the DAW. Could you please provide me (us) with more details? Thank you very much!

tttapa commented 3 years ago

Basically, you combine the PBValue and the PBAbsoluteEncoder::setValue() method. The former just receives the position from the DAW, and you use that position to update the value of the encoder.

I don't have time to try this out myself right now.

d1One commented 2 years ago

Basically, you combine the PBValue and the PBAbsoluteEncoder::setValue() method. The former just receives the position from the DAW, and you use that position to update the value of the encoder.

I don't have time to try this out myself right now.

Any chance you could provide a small example?

simoniddqd commented 1 year ago

The former just receives the position from the DAW, and you use that position to update the value of the encoder.

It seems my code resets the value position before it can get read by the pb.getValue function in void loop