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

Active sensingv2 #155

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Active sensingv2 #155

wants to merge 6 commits into from

Conversation

lathoub
Copy link
Collaborator

@lathoub lathoub commented May 17, 2020

Active Sensing

2 types of Active Sensing, receiving and sensing of Active Sensing. Work has been based on re-reading literature and online MIDI implementation manuals from various manufacturars.

receiving Active Sensing

Once an Active Sensing message is received, the unit will begin monitoring the interval between all subsequent messages. If there is an interval of Settings::ReceiverActiveSensingTimeout ms or longer between messages while monitoring is active, the ErrorActiveSensingTimeout flag is set and the Error callback is called. It is up to the application to stop All Notes. The unit will then stop monitoring the message interval.

Turn on receiving Active Sensing using the overwritten UseReceiverActiveSensing in Settings

Sending Active Sensing

ActiveSensing is send Settings::ActiveSensingPeriodicity ms after the last command

Turn on sending Active Sensing using the overwritten UseSenderActiveSensing in Settings

Note:

Both UseReceiverActiveSensing and UseSenderActiveSensing can't be set to true, it's one or the other.

Serial Settings Example (Hairless MIDI)

Add Hairless example (but currently unrest as hairless is not supported on MaxOS Catalina 64bit)

lathoub added 4 commits May 16, 2020 16:35
reworked ActiveSensing using input from a variety of device MIDI Implementation manuals (Roland,  KORG, Yamaha) found on the internet.

Receiving ActiveSensing:
Once an Active Sensing message is received, the unit will begin monitoring the intervalbetween all subsequent messages. If there is an interval of ActiveSensingPeriodicity ms or longer betweenmessages while monitoring is active, the same processing as when All Sound Off, All Notes Off,and Reset All Controllers messages are received will be carried out. The unit will then stopmonitoring the message interval.

Sending ActiveSensing: send x ms after the last sent command
@coveralls
Copy link

coveralls commented May 17, 2020

Coverage Status

Coverage remained the same at 89.363% when pulling 37ef1d4 on ActiveSensingv2 into ff3052c on master.

@lathoub
Copy link
Collaborator Author

lathoub commented May 18, 2020

Good reading on Active Sensing

Another reading on impact of Active Sensing (in Dutch)

The load that Active Sensing would mean for your midi data is heavily overestimated by some. First of all, the command 'only' is sent three times per second and only consists of 1 byte. But more important is that it is only sent when there is no other midi activity. As soon as notes or other midi data pass through the cable, Active Sensing stops. It is therefore virtually impossible for delays in your midi data to be caused by Active Sensing. It is only there when it is needed. At times when other mididata go through the cable, there is midi-activity and therefore Active Sensing is superfluous. After the last midi event has been sent, Active Sensing starts rattling again. Until another midi event is sent.

Translated with www.DeepL.com/Translator (free version)

@franky47
Copy link
Member

The terminology for timeouts could be improved, it's not an error but a valid event of the lifetime of the MIDI stream. An error would be if something unparsable or very much unexpected happened, like one of the few unreachable places in the parser.

@lathoub
Copy link
Collaborator Author

lathoub commented May 18, 2020

The terminology for timeouts could be improved, it's not an error...

I kinda agree, the naming for the handler Error came out of a Parsing error. In the case of ActiveSensing Exception would be better. A start would to rename

static const uint8_t ErrorActiveSensingTimeout = 1;

to ActiveSensingTimeoutException (but the handler is still called ErrorCallback , it would have been better to have called it the ExceptionHandler).

Also the Exception-bits should be in an enum....

@franky47
Copy link
Member

franky47 commented May 18, 2020

Could it be its own event ?

void onActiveSensingTimeout()
{
  // Loss of MIDI activity detected
}

MIDI.handleActiveSensingTimeout(onActiveSensingTimeout)

This would make it opt-in and avoid writing additional code to figure out the type of error or exception in a generic handler.

@lathoub
Copy link
Collaborator Author

lathoub commented May 18, 2020

Just implemented the 'having it's own handler' and its memory neutral (balanced the additional bit checking code calling the error handler)

@lathoub
Copy link
Collaborator Author

lathoub commented May 18, 2020

How do we call this thing?

using ActiveSensingTimeoutCallback = void (*)(bool active);

or

using ActiveSensingExceptionCallback = void (*)(bool timedOut);

...

@franky47
Copy link
Member

I'd go for active, false on timeout and true when stream resumes activity.

lathoub and others added 2 commits May 23, 2020 10:51
removed ErrorActiveSensingTimeout
return mReceiverActiveSensingActive in SensingTimeout Handler:
- true when all is OK and active sensing is received on time
- false when active sensing is **not** received on tome
@lathoub
Copy link
Collaborator Author

lathoub commented May 23, 2020

false on timeout and true when stream resumes activity.

Active Sensing tested (both receiver and sending)

@lathoub
Copy link
Collaborator Author

lathoub commented Dec 28, 2020

@franky47 This has been out for a while (a reworked and optimized version of the current ActiveSensing), is this something to be considered for the next update?

@franky47
Copy link
Member

Yes we can include this in the next release.

@franky47 franky47 added this to the 5.0.3 milestone Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants