Skip to content

Commit

Permalink
Fix endpoint connection bugs introduced with auto-reconnect
Browse files Browse the repository at this point in the history
  • Loading branch information
@Psychlist1972
Psychlist1972 committed Apr 26, 2024
1 parent 008d19f commit 37a639f
Show file tree
Hide file tree
Showing 18 changed files with 399 additions and 163 deletions.
4 changes: 2 additions & 2 deletions build/replace_running_service_x64.bat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ copy /Y %buildoutput%\MidiSrv.exe %servicepath%
copy /Y %buildoutput%\Midi2.*Abstraction.dll %servicepath%
copy /Y %buildoutput%\Midi2.*Transform.dll %servicepath%

echo Windows.Devices.Midi2.dll
echo API impl
copy /Y %buildoutput%\*.Devices.Midi2.dll %apipath%
echo Windows.Devices.Midi2.pri
echo API pri
copy /Y %buildoutput%\*.Devices.Midi2.pri %apipath%


Expand Down
2 changes: 1 addition & 1 deletion build/staging/version/BundleInfo.wxi
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Include>
<?define SetupVersionName="Developer Preview 6 arm64" ?>
<?define SetupVersionNumber="1.0.24113.0059" ?>
<?define SetupVersionNumber="1.0.24117.0049" ?>
</Include>
4 changes: 2 additions & 2 deletions src/api/Client/Midi2Client-Projection/nuget/Microsoft.Devices.Midi2.nuspec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
<package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
<metadata>
<id>Microsoft.Devices.Midi2</id>
<version>1.0.0-preview.6-0197</version>
<authors>Microsoft Corporation</authors>
<version>1.0.0-preview.6-0198</version>
<authors>Microsoft</authors>
<description>Windows MIDI Services SDK. Minimum package necessary to use Windows MIDI Services from an app on a PC that has Windows MIDI Services installed.</description>
<license type="expression">MIT</license>
<projectUrl>https://github.com/microsoft/midi/</projectUrl>
Expand Down
112 changes: 81 additions & 31 deletions src/api/Client/Midi2Client/MidiEndpointConnection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ namespace MIDI_CPP_NAMESPACE::implementation
_Use_decl_annotations_
HRESULT MidiEndpointConnection::Callback(PVOID data, UINT size, LONGLONG timestamp, LONGLONG)
{
internal::LogInfo(__FUNCTION__, L"Message Received");
// internal::LogInfo(__FUNCTION__, L"Message Received in API callback");

try
{
// one copy of the event args for this gets sent to all listeners and the main event
auto args = winrt::make_self<midi2::implementation::MidiMessageReceivedEventArgs>(data, size, timestamp);
auto args = winrt::make_self<implementation::MidiMessageReceivedEventArgs>(data, size, timestamp);

// we failed to create the event args
if (args == nullptr)
Expand Down Expand Up @@ -63,12 +63,23 @@ namespace MIDI_CPP_NAMESPACE::implementation
}
catch (winrt::hresult_error const& ex)
{
internal::LogHresultError(__FUNCTION__, L"hresult exception calling message handlers", ex);
internal::LogHresultError(__FUNCTION__, L"hresult exception handling received messages", ex);

return E_FAIL;
}
}
catch (std::exception const& ex)
{
internal::LogStandardExceptionError(__FUNCTION__, L"hresult exception handling received messages", ex);

return E_FAIL;
}
catch (...)
{
internal::LogGeneralError(__FUNCTION__, L"Exception handling received message");

return E_FAIL;
}
}


_Use_decl_annotations_
Expand All @@ -81,7 +92,14 @@ namespace MIDI_CPP_NAMESPACE::implementation
bool autoReconnect
)
{
internal::LogInfo(__FUNCTION__, L"Internal Initialize ");
internal::LogInfo(__FUNCTION__, L"Enter");

if (serviceAbstraction == nullptr)
{
internal::LogGeneralError(__FUNCTION__, L"Error initializing endpoint. Service abstraction is null");

return false;
}

try
{
Expand All @@ -94,7 +112,6 @@ namespace MIDI_CPP_NAMESPACE::implementation

m_connectionSettings = connectionSettings;
m_autoReconnect = autoReconnect;


return true;
}
Expand All @@ -110,19 +127,29 @@ namespace MIDI_CPP_NAMESPACE::implementation
_Use_decl_annotations_
bool MidiEndpointConnection::InternalOpen()
{
internal::LogInfo(__FUNCTION__, L"Connection Open ");
internal::LogInfo(__FUNCTION__, L"Enter");

if (m_endpointAbstraction == nullptr)
{
internal::LogGeneralError(__FUNCTION__, L"Failed to open connection. endpoint abstraction is null");

return false;
}

DWORD mmcssTaskId{};

LPCWSTR connectionSettingsJsonString = nullptr;

if (m_connectionSettings != nullptr)
{
connectionSettingsJsonString = m_connectionSettings.SettingsJson().c_str();
internal::LogInfo(__FUNCTION__, L"Connection settings were specified. Including JSON in service call.");

connectionSettingsJsonString = static_cast<LPCWSTR>(m_connectionSettings.SettingsJson().c_str());
}

ABSTRACTIONCREATIONPARAMS abstractionCreationParams{ MidiDataFormat_UMP, connectionSettingsJsonString };
ABSTRACTIONCREATIONPARAMS abstractionCreationParams{ };
abstractionCreationParams.DataFormat = MidiDataFormat_UMP;
abstractionCreationParams.InstanceConfigurationJsonData = connectionSettingsJsonString;

auto result = m_endpointAbstraction->Initialize(
(LPCWSTR)(EndpointDeviceId().c_str()),
Expand All @@ -135,6 +162,8 @@ namespace MIDI_CPP_NAMESPACE::implementation

if (SUCCEEDED(result))
{
internal::LogInfo(__FUNCTION__, L"Endpoint abstraction successfully initialized");

m_isOpen = true;
m_closeHasBeenCalled = false;
}
Expand All @@ -143,22 +172,18 @@ namespace MIDI_CPP_NAMESPACE::implementation
internal::LogHresultError(__FUNCTION__, L"Failed to open connection", result);
}

internal::LogInfo(__FUNCTION__, L"Connection Opened");

return true;
}

// this does all the reconnection except for plugin init
_Use_decl_annotations_
bool MidiEndpointConnection::InternalReopenAfterDisconnect()
{
internal::LogInfo(__FUNCTION__, L"Connection Reopen ");
internal::LogInfo(__FUNCTION__, L"Enter");

// Activate the endpoint for this device. Will fail if the device is not a BiDi device
if (!ActivateMidiStream())
{
internal::LogGeneralError(__FUNCTION__, L"Could not activate MIDI Stream");

return false;
}
if (!ActivateMidiStream()) return false;

return InternalOpen();
}
Expand All @@ -167,19 +192,12 @@ namespace MIDI_CPP_NAMESPACE::implementation
_Use_decl_annotations_
bool MidiEndpointConnection::Open()
{
internal::LogInfo(__FUNCTION__, L"Connection Open ");

// Activate the endpoint for this device. Will fail if the device is not a BiDi device
if (!ActivateMidiStream())
{
internal::LogGeneralError(__FUNCTION__, L"Could not activate MIDI Stream");

return false;
}

internal::LogInfo(__FUNCTION__, L"Enter");

if (!IsOpen())
{
if (!ActivateMidiStream()) return false;

if (m_endpointAbstraction != nullptr)
{
try
Expand All @@ -200,6 +218,8 @@ namespace MIDI_CPP_NAMESPACE::implementation
}
else
{
internal::LogGeneralError(__FUNCTION__, L"InternalOpen() returned false.");

return false;
}
}
Expand Down Expand Up @@ -229,6 +249,8 @@ namespace MIDI_CPP_NAMESPACE::implementation
}
else
{
internal::LogInfo(__FUNCTION__, L"Connection already open");

// already open. Just return true here. Not fatal in any way, so no error
return true;
}
Expand All @@ -238,7 +260,7 @@ namespace MIDI_CPP_NAMESPACE::implementation

void MidiEndpointConnection::InternalClose()
{
internal::LogInfo(__FUNCTION__, L"Connection Close");
internal::LogInfo(__FUNCTION__, L"Enter");

if (m_closeHasBeenCalled) return;

Expand All @@ -257,6 +279,8 @@ namespace MIDI_CPP_NAMESPACE::implementation
{
internal::LogGeneralError(__FUNCTION__, L"Exception on close");
}

internal::LogInfo(__FUNCTION__, L"Complete");
}

MidiEndpointConnection::~MidiEndpointConnection()
Expand All @@ -270,29 +294,55 @@ namespace MIDI_CPP_NAMESPACE::implementation
_Use_decl_annotations_
bool MidiEndpointConnection::DeactivateMidiStream()
{
internal::LogInfo(__FUNCTION__, L"Deactivating MIDI Stream");
internal::LogInfo(__FUNCTION__, L"Enter");

if (m_endpointAbstraction != nullptr)
{
// todo: some error / hresult handling here

m_endpointAbstraction->Cleanup();
auto hr = m_endpointAbstraction->Cleanup();

if (FAILED(hr))
{
internal::LogHresultError(__FUNCTION__, L"Failed HRESULT cleaning up endpoint abstraction", hr);

return false;
}

m_endpointAbstraction == nullptr;
}

internal::LogInfo(__FUNCTION__, L"Stream deactivated");

return true;
}


_Use_decl_annotations_
bool MidiEndpointConnection::ActivateMidiStream() noexcept
{
internal::LogInfo(__FUNCTION__, L"Activating MIDI Stream");
internal::LogInfo(__FUNCTION__, L"Enter");

if (m_serviceAbstraction == nullptr)
{
internal::LogGeneralError(__FUNCTION__, L"Service abstraction is null.");

return false;
}

try
{
winrt::check_hresult(m_serviceAbstraction->Activate(__uuidof(IMidiBiDi), (void**) &m_endpointAbstraction));

if (m_endpointAbstraction == nullptr)
{
internal::LogGeneralError(__FUNCTION__, L"Endpoint Abstraction failed to Activate and return null.");

return false;
}

internal::LogInfo(__FUNCTION__, L"Activated IMidiBiDi");

return true;
}
catch (winrt::hresult_error const& ex)
Expand Down
74 changes: 47 additions & 27 deletions src/api/Client/Midi2Client/MidiEndpointConnection_AutoReconnect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,57 +9,75 @@
#include "pch.h"
#include "MidiEndpointConnection.h"


// auto deviceAddedHandler = TypedEventHandler<DeviceWatcher, DeviceInformation>(this, &CMidi2KSMidiEndpointManager::OnDeviceAdded);
// auto deviceUpdatedHandler = TypedEventHandler<DeviceWatcher, DeviceInformationUpdate>(this, &CMidi2KSMidiEndpointManager::OnDeviceUpdated);
// auto deviceRemovedHandler = TypedEventHandler<DeviceWatcher, DeviceInformationUpdate>(this, &CMidi2KSMidiEndpointManager::OnDeviceRemoved);
// m_DeviceAdded = m_Watcher.Added(winrt::auto_revoke, deviceAddedHandler);
// m_DeviceUpdated = m_Watcher.Updated(winrt::auto_revoke, deviceUpdatedHandler);
// m_DeviceRemoved = m_Watcher.Removed(winrt::auto_revoke, deviceRemovedHandler);


namespace MIDI_CPP_NAMESPACE::implementation
{
// TODO: An optimization here would be to host the watcher at the session level
// and simply have it filter based on the Ids it sees in the update messages. Then
// include internal functions in this class that it would call
_Use_decl_annotations_
bool MidiEndpointConnection::StartDeviceWatcher()
{
// start the device watcher for the specific Id
winrt::hstring deviceSelector(
L"Id:=\"" + m_endpointDeviceId + L"\" AND System.Devices.InterfaceEnabled:=System.StructuredQueryType.Boolean#True");
internal::LogInfo(__FUNCTION__, L"Enter");

m_autoReconnectDeviceWatcher = enumeration::DeviceInformation::CreateWatcher(deviceSelector);

if (m_autoReconnectDeviceWatcher != nullptr)
try
{
if (m_autoReconnectDeviceWatcher.Status() == enumeration::DeviceWatcherStatus::Stopped)
{
auto deviceAddedHandler = foundation::TypedEventHandler<enumeration::DeviceWatcher, enumeration::DeviceInformation>(this, &MidiEndpointConnection::DeviceAddedHandler);
auto deviceUpdatedHandler = foundation::TypedEventHandler<enumeration::DeviceWatcher, enumeration::DeviceInformationUpdate>(this, &MidiEndpointConnection::DeviceUpdatedHandler);
auto deviceRemovedHandler = foundation::TypedEventHandler<enumeration::DeviceWatcher, enumeration::DeviceInformationUpdate>(this, &MidiEndpointConnection::DeviceRemovedHandler);
// start the device watcher for the specific Id
winrt::hstring deviceSelector(
L"System.Devices.DeviceInstanceId:=\"" + m_endpointDeviceId + L"\" AND System.Devices.InterfaceEnabled:=System.StructuredQueryType.Boolean#True");

internal::LogInfo(__FUNCTION__, deviceSelector.c_str());

m_autoReconnectDeviceWatcherAddedRevoker = m_autoReconnectDeviceWatcher.Added(winrt::auto_revoke, deviceAddedHandler);
m_autoReconnectDeviceWatcherUpdatedRevoker = m_autoReconnectDeviceWatcher.Updated(winrt::auto_revoke, deviceUpdatedHandler);
m_autoReconnectDeviceWatcherRemovedRevoker = m_autoReconnectDeviceWatcher.Removed(winrt::auto_revoke, deviceRemovedHandler);

m_autoReconnectDeviceWatcher.Start();
m_autoReconnectDeviceWatcher = enumeration::DeviceInformation::CreateWatcher(deviceSelector);

return true;
if (m_autoReconnectDeviceWatcher != nullptr)
{
if (m_autoReconnectDeviceWatcher.Status() == enumeration::DeviceWatcherStatus::Stopped)
{
auto deviceAddedHandler = foundation::TypedEventHandler<enumeration::DeviceWatcher, enumeration::DeviceInformation>(this, &MidiEndpointConnection::DeviceAddedHandler);
auto deviceUpdatedHandler = foundation::TypedEventHandler<enumeration::DeviceWatcher, enumeration::DeviceInformationUpdate>(this, &MidiEndpointConnection::DeviceUpdatedHandler);
auto deviceRemovedHandler = foundation::TypedEventHandler<enumeration::DeviceWatcher, enumeration::DeviceInformationUpdate>(this, &MidiEndpointConnection::DeviceRemovedHandler);

m_autoReconnectDeviceWatcherAddedRevoker = m_autoReconnectDeviceWatcher.Added(winrt::auto_revoke, deviceAddedHandler);
m_autoReconnectDeviceWatcherUpdatedRevoker = m_autoReconnectDeviceWatcher.Updated(winrt::auto_revoke, deviceUpdatedHandler);
m_autoReconnectDeviceWatcherRemovedRevoker = m_autoReconnectDeviceWatcher.Removed(winrt::auto_revoke, deviceRemovedHandler);

m_autoReconnectDeviceWatcher.Start();

return true;
}
else
{
// can only start the watcher when it's in a stopped state
return false;
}
}
else
{
// can only start the watcher when it's in a stopped state
return false;
}

}
else
catch (winrt::hresult_error hr)
{
internal::LogHresultError(__FUNCTION__, L"HRESULT Exception starting device watcher", hr);

return false;
}
catch (...)
{
internal::LogGeneralError(__FUNCTION__, L"Exception starting device watcher");

return false;
}

}

_Use_decl_annotations_
bool MidiEndpointConnection::StopDeviceWatcher()
{
internal::LogInfo(__FUNCTION__, L"Enter");

if (m_autoReconnectDeviceWatcher != nullptr)
{
m_autoReconnectDeviceWatcher.Stop();
Expand Down Expand Up @@ -92,6 +110,8 @@ namespace MIDI_CPP_NAMESPACE::implementation
// if not m_isOpen, then perform all the opening logic again
if (!m_isOpen)
{
internal::LogInfo(__FUNCTION__, L"Reopening after disconnect (m_isOpen == false)");

InternalReopenAfterDisconnect();
}

Expand Down
Loading

0 comments on commit 37a639f

Please sign in to comment.