Fix some of the issues with bank handling (#461)

* Fix for Issue #457 - changed m_nNumLoadedBanks to be m_nNumHighestBank and updated functionality in menus accordingly.

* Fix for Issue #460 implements MIDI Bank Select MSB/LSB based on PR #395 submitted by @abscisys

* Fix for Issue #458 to not show blank banks in the menus.  Also handles MIDI bank select messages and won't change banks if the final selected bank isn't loaded.

* Firm up bank validity handling slightly.

src/mididevice.cpp

@@ -36,7 +36,7 @@ LOGMODULE ("mididevice");
 #define MIDI_AFTERTOUCH		0b1010			// TODO
 #define MIDI_CHANNEL_AFTERTOUCH 0b1101   // right now Synth_Dexed just manage Channel Aftertouch not Polyphonic AT -> 0b1010
 #define MIDI_CONTROL_CHANGE	0b1011
-	#define MIDI_CC_BANK_SELECT_MSB		0	// TODO
+	#define MIDI_CC_BANK_SELECT_MSB		0
 	#define MIDI_CC_MODULATION			1
 	#define MIDI_CC_BREATH_CONTROLLER	2 
 	#define MIDI_CC_FOOT_PEDAL 		4
@@ -283,6 +283,10 @@ void CMIDIDevice::MIDIMessageHandler (const u8 *pMessage, size_t nLength, unsign
 							m_pSynthesizer->SetPan (pMessage[2], nTG);
 							break;
 		
+						case MIDI_CC_BANK_SELECT_MSB:
+							m_pSynthesizer->BankSelectMSB (pMessage[2], nTG);
+							break;
+		
 						case MIDI_CC_BANK_SELECT_LSB:
 							m_pSynthesizer->BankSelectLSB (pMessage[2], nTG);
 							break;

src/minidexed.cpp

@@ -61,6 +61,7 @@ CMiniDexed::CMiniDexed (CConfig *pConfig, CInterruptSystem *pInterrupt,
 	for (unsigned i = 0; i < CConfig::ToneGenerators; i++)
 	{
 		m_nVoiceBankID[i] = 0;
+		m_nVoiceBankIDMSB[i] = 0;
 		m_nProgram[i] = 0;
 		m_nVolume[i] = 100;
 		m_nPan[i] = 64;
@@ -365,14 +366,47 @@ CSysExFileLoader *CMiniDexed::GetSysExFileLoader (void)
 	return &m_SysExFileLoader;
 }
 
+void CMiniDexed::BankSelect (unsigned nBank, unsigned nTG)
+{
+	nBank=constrain((int)nBank,0,16383);
+
+	assert (nTG < CConfig::ToneGenerators);
+	
+	if (GetSysExFileLoader ()->IsValidBank(nBank))
+	{
+		// Only change if we have the bank loaded
+		m_nVoiceBankID[nTG] = nBank;
+
+		m_UI.ParameterChanged ();
+	}
+}
+
+void CMiniDexed::BankSelectMSB (unsigned nBankMSB, unsigned nTG)
+{
+	nBankMSB=constrain((int)nBankMSB,0,127);
+
+	assert (nTG < CConfig::ToneGenerators);
+	// MIDI Spec 1.0 "BANK SELECT" states:
+	//   "The transmitter must transmit the MSB and LSB as a pair,
+	//   and the Program Change must be sent immediately after
+	//   the Bank Select pair."
+	//
+	// So it isn't possible to validate the selected bank ID until
+	// we receive both MSB and LSB so just store the MSB for now.
+	m_nVoiceBankIDMSB[nTG] = nBankMSB;
+}
+
 void CMiniDexed::BankSelectLSB (unsigned nBankLSB, unsigned nTG)
 {
 	nBankLSB=constrain((int)nBankLSB,0,127);
 
 	assert (nTG < CConfig::ToneGenerators);
-	m_nVoiceBankID[nTG] = nBankLSB;
+	unsigned nBank = m_nVoiceBankID[nTG];
+	unsigned nBankMSB = m_nVoiceBankIDMSB[nTG];
+	nBank = (nBankMSB << 7) + nBankLSB;
 
-	m_UI.ParameterChanged ();
+	// Now should have both MSB and LSB so enable the BankSelect
+	BankSelect(nBank, nTG);
 }
 
 void CMiniDexed::ProgramChange (unsigned nProgram, unsigned nTG)
@@ -717,7 +751,9 @@ void CMiniDexed::SetTGParameter (TTGParameter Parameter, int nValue, unsigned nT
 
 	switch (Parameter)
 	{
-	case TGParameterVoiceBank:	BankSelectLSB (nValue, nTG);	break;
+	case TGParameterVoiceBank:	BankSelect (nValue, nTG);	break;
+	case TGParameterVoiceBankMSB:	BankSelectMSB (nValue, nTG);	break;
+	case TGParameterVoiceBankLSB:	BankSelectLSB (nValue, nTG);	break;
 	case TGParameterProgram:	ProgramChange (nValue, nTG);	break;
 	case TGParameterVolume:		SetVolume (nValue, nTG);	break;
 	case TGParameterPan:		SetPan (nValue, nTG);		break;
@@ -771,6 +807,8 @@ int CMiniDexed::GetTGParameter (TTGParameter Parameter, unsigned nTG)
 	switch (Parameter)
 	{
 	case TGParameterVoiceBank:	return m_nVoiceBankID[nTG];
+	case TGParameterVoiceBankMSB:	return m_nVoiceBankID[nTG] >> 7;
+	case TGParameterVoiceBankLSB:	return m_nVoiceBankID[nTG] & 0x7F;
 	case TGParameterProgram:	return m_nProgram[nTG];
 	case TGParameterVolume:		return m_nVolume[nTG];
 	case TGParameterPan:		return m_nPan[nTG];
@@ -1445,7 +1483,7 @@ void CMiniDexed::LoadPerformanceParameters(void)
 	for (unsigned nTG = 0; nTG < CConfig::ToneGenerators; nTG++)
 		{
 			
-			BankSelectLSB (m_PerformanceConfig.GetBankNumber (nTG), nTG);
+			BankSelect (m_PerformanceConfig.GetBankNumber (nTG), nTG);
 			ProgramChange (m_PerformanceConfig.GetVoiceNumber (nTG), nTG);
 			SetMIDIChannel (m_PerformanceConfig.GetMIDIChannel (nTG), nTG);
 			SetVolume (m_PerformanceConfig.GetVolume (nTG), nTG);

src/minidexed.h

@@ -63,6 +63,8 @@ public:
 
 	CSysExFileLoader *GetSysExFileLoader (void);
 
+	void BankSelect    (unsigned nBank, unsigned nTG);
+	void BankSelectMSB (unsigned nBankMSB, unsigned nTG);
 	void BankSelectLSB (unsigned nBankLSB, unsigned nTG);
 	void ProgramChange (unsigned nProgram, unsigned nTG);
 	void SetVolume (unsigned nVolume, unsigned nTG);
@@ -149,6 +151,8 @@ public:
 	enum TTGParameter
 	{
 		TGParameterVoiceBank,
+		TGParameterVoiceBankMSB,
+		TGParameterVoiceBankLSB,
 		TGParameterProgram,
 		TGParameterVolume,
 		TGParameterPan,
@@ -227,6 +231,7 @@ private:
 	CDexedAdapter *m_pTG[CConfig::ToneGenerators];
 
 	unsigned m_nVoiceBankID[CConfig::ToneGenerators];
+	unsigned m_nVoiceBankIDMSB[CConfig::ToneGenerators];
 	unsigned m_nProgram[CConfig::ToneGenerators];
 	unsigned m_nVolume[CConfig::ToneGenerators];
 	unsigned m_nPan[CConfig::ToneGenerators];

src/sysexfileloader.cpp

@@ -65,7 +65,7 @@ CSysExFileLoader::CSysExFileLoader (const char *pDirName)
 :	m_DirName (pDirName)
 {
 	m_DirName += "/voice";
-	m_nNumLoadedBanks = 0;
+	m_nNumHighestBank = 0;
 
 	for (unsigned i = 0; i <= MaxVoiceBankID; i++)
 	{
@@ -83,7 +83,7 @@ CSysExFileLoader::~CSysExFileLoader (void)
 
 void CSysExFileLoader::Load (void)
 {
-	m_nNumLoadedBanks = 0;
+	m_nNumHighestBank = 0;
 
     DIR *pDirectory = opendir (m_DirName.c_str ());
 	if (!pDirectory)
@@ -141,7 +141,11 @@ void CSysExFileLoader::Load (void)
 				LOGDBG ("Bank #%u successfully loaded", nBank);
 
 				m_BankFileName[nBank] = pEntry->d_name;
-				m_nNumLoadedBanks++;
+				if (nBank > m_nNumHighestBank)
+				{
+					// This is the bank ID of the highest loaded bank
+					m_nNumHighestBank = nBank;
+				}
 			}
 			else
 			{
@@ -188,9 +192,71 @@ std::string CSysExFileLoader::GetBankName (unsigned nBankID)
 	return "NO NAME";
 }
 
-unsigned CSysExFileLoader::GetNumLoadedBanks (void)
+unsigned CSysExFileLoader::GetNextBankUp (unsigned nBankID)
+{
+	// Find the next loaded bank "up" from the provided bank ID
+	for (unsigned id=nBankID+1; id <= m_nNumHighestBank; id++)
+	{
+		if (IsValidBank(id))
+		{
+			return id;
+		}
+	}
+	
+	// Handle wrap-around
+	for (unsigned id=0; id<nBankID; id++)
 	{
-	return m_nNumLoadedBanks;
+		if (IsValidBank(id))
+		{
+			return id;
+		}
+	}
+	
+	// If we get here there are no other banks!
+	return nBankID;
+}
+
+unsigned CSysExFileLoader::GetNextBankDown (unsigned nBankID)
+{
+	// Find the next loaded bank "down" from the provided bank ID
+	for (int id=((int)nBankID)-1; id >= 0; id--)
+	{
+		if (IsValidBank((unsigned)id))
+		{
+			return id;
+		}
+	}
+
+	// Handle wrap-around
+	for (unsigned id=m_nNumHighestBank; id>nBankID; id--)
+	{
+		if (IsValidBank(id))
+		{
+			return id;
+		}
+	}
+	
+	// If we get here there are no other banks!
+	return nBankID;
+}
+
+bool CSysExFileLoader::IsValidBank (unsigned nBankID)
+{
+	if (m_pVoiceBank[nBankID])
+	{
+		// Use a valid "status start/end" as an indicator of a valid loaded bank
+		if ((m_pVoiceBank[nBankID]->StatusStart == 0xF0) &&
+			(m_pVoiceBank[nBankID]->StatusEnd == 0xF7))
+		{
+			return true;
+		}
+	}
+	return false;
+}
+
+unsigned CSysExFileLoader::GetNumHighestBank (void)
+{
+	return m_nNumHighestBank;
 }
 
 void CSysExFileLoader::GetVoice (unsigned nBankID, unsigned nVoiceID, uint8_t *pVoiceData)
@@ -198,7 +264,7 @@ void CSysExFileLoader::GetVoice (unsigned nBankID, unsigned nVoiceID, uint8_t *p
 	if (   nBankID <= MaxVoiceBankID
 	    && nVoiceID <= VoicesPerBank)
 	{
-		if (m_pVoiceBank[nBankID])
+		if (IsValidBank(nBankID))
 		{
 			DecodePackedVoice (m_pVoiceBank[nBankID]->Voice[nVoiceID], pVoiceData);
 

src/sysexfileloader.h

@@ -29,7 +29,7 @@
 class CSysExFileLoader		// Loader for DX7 .syx files
 {
 public:
-	static const unsigned MaxVoiceBankID = 16383;
+	static const unsigned MaxVoiceBankID = 16383; // i.e. 14-bit MSB/LSB value between 0 and 16383
 	static const unsigned VoicesPerBank = 32;
 	static const size_t SizePackedVoice = 128;
 	static const size_t SizeSingleVoice = 156;
@@ -56,10 +56,13 @@ public:
 
 	void Load (void);
 
-	std::string GetBankName (unsigned nBankID);	// 0 .. 127
+	std::string GetBankName (unsigned nBankID);	// 0 .. MaxVoiceBankID
-	unsigned GetNumLoadedBanks (); // 0 .. MaxVoiceBankID
+	unsigned GetNumHighestBank (); // 0 .. MaxVoiceBankID
+	bool     IsValidBank (unsigned nBankID);
+	unsigned GetNextBankUp (unsigned nBankID);
+	unsigned GetNextBankDown (unsigned nBankID);
 
-	void GetVoice (unsigned nBankID,		// 0 .. 127
+	void GetVoice (unsigned nBankID,		// 0 .. MaxVoiceBankID
 		       unsigned nVoiceID,		// 0 .. 31
 		       uint8_t *pVoiceData);		// returns unpacked format (156 bytes)
 
@@ -69,7 +72,7 @@ private:
 private:
 	std::string m_DirName;
 	
-	unsigned m_nNumLoadedBanks;
+	unsigned m_nNumHighestBank;
 
 	TVoiceBank *m_pVoiceBank[MaxVoiceBankID+1];
 	std::string m_BankFileName[MaxVoiceBankID+1];

src/uimenu.cpp

@@ -486,7 +486,6 @@ void CUIMenu::EditGlobalParameter (CUIMenu *pUIMenu, TMenuEvent Event)
 void CUIMenu::EditVoiceBankNumber (CUIMenu *pUIMenu, TMenuEvent Event)
 {
 	unsigned nTG = pUIMenu->m_nMenuStackParameter[pUIMenu->m_nCurrentMenuDepth-1];
-	int nLoadedBanks = pUIMenu->m_pMiniDexed->GetSysExFileLoader ()->GetNumLoadedBanks();
 
 	int nValue = pUIMenu->m_pMiniDexed->GetTGParameter (CMiniDexed::TGParameterVoiceBank, nTG);
 
@@ -496,19 +495,13 @@ void CUIMenu::EditVoiceBankNumber (CUIMenu *pUIMenu, TMenuEvent Event)
 		break;
 
 	case MenuEventStepDown:
-		if (--nValue < 0)
+		nValue = pUIMenu->m_pMiniDexed->GetSysExFileLoader ()->GetNextBankDown(nValue);
-		{
-			nValue = 0;
-		}
 		pUIMenu->m_pMiniDexed->SetTGParameter (
 			CMiniDexed::TGParameterVoiceBank, nValue, nTG);
 		break;
 
 	case MenuEventStepUp:
-		if (++nValue > (int) nLoadedBanks-1)
+		nValue = pUIMenu->m_pMiniDexed->GetSysExFileLoader ()->GetNextBankUp(nValue);
-		{
-			nValue = nLoadedBanks-1;
-		}
 		pUIMenu->m_pMiniDexed->SetTGParameter (
 			CMiniDexed::TGParameterVoiceBank, nValue, nTG);
 		break;
@@ -537,7 +530,7 @@ void CUIMenu::EditVoiceBankNumber (CUIMenu *pUIMenu, TMenuEvent Event)
 void CUIMenu::EditProgramNumber (CUIMenu *pUIMenu, TMenuEvent Event)
 {
 	unsigned nTG = pUIMenu->m_nMenuStackParameter[pUIMenu->m_nCurrentMenuDepth-1];
-	int nLoadedBanks = pUIMenu->m_pMiniDexed->GetSysExFileLoader ()->GetNumLoadedBanks();
+	int nHighestBank = pUIMenu->m_pMiniDexed->GetSysExFileLoader ()->GetNumHighestBank();
 
 	int nValue = pUIMenu->m_pMiniDexed->GetTGParameter (CMiniDexed::TGParameterProgram, nTG);
 
@@ -555,7 +548,7 @@ void CUIMenu::EditProgramNumber (CUIMenu *pUIMenu, TMenuEvent Event)
 			if (--nVB < 0)
 			{
 				// Wrap around to last loaded bank
-				nVB = nLoadedBanks-1;
+				nVB = nHighestBank;
 			}
 			pUIMenu->m_pMiniDexed->SetTGParameter (CMiniDexed::TGParameterVoiceBank, nVB, nTG);
 		}
@@ -568,7 +561,7 @@ void CUIMenu::EditProgramNumber (CUIMenu *pUIMenu, TMenuEvent Event)
 			// Switch up a voice bank and reset to voice 0
 			nValue = 0;
 			int nVB = pUIMenu->m_pMiniDexed->GetTGParameter(CMiniDexed::TGParameterVoiceBank, nTG);
-			if (++nVB > (int) nLoadedBanks-1)
+			if (++nVB > (int) nHighestBank)
 			{
 				// Wrap around to first bank
 				nVB = 0;