SwitchMultiPos not correctly accounting for PIN_NC and instead sends the PIN_NC value

[kbastronomics]

In working with SwitchMultiPos we have a 8 position switch with pin 1 not connected.
when we switch between positions on the switch the not connected pin gets sent between each position (ie on a non-shorting switch)

so for example, say an 8 position switch and position 1 is not connected.
you start on position 2,
you move to position 3.
soon as position 2 disconnects position 1 (this PIN_NC pin) is sent so in DCS commands coming from the switch you see this
INS_SW 2
INS_SW 1
INS_SW 3

and if you continue sweeping the switch you get
INS_SW 1
INS_SW 4
INS_SW 1
INS_SW 5
INS_SW 1
INS_SW 6
INS_SW 1
INS_SW 7
INS_SW 1
INS_SW 8
INS_SW 1
INS_SW 7
and so on

from looking at the code for SwitchMultiPos

	class SwitchMultiPosT : PollingInput, public ResettableInput
	{
	private:
		const char* msg_;
		const byte* pins_;
		char numberOfPins_;
		char lastState_;
		bool reverse_;
		char readState() {
			unsigned char ncPinIdx = lastState_;
			for (unsigned char i=0; i<numberOfPins_; i++) {
				if( pins_[i] == PIN_NC)
					ncPinIdx = i;
				else
				{
					if (digitalRead(pins_[i]) == LOW && reverse_ == false) return i;
					else if (digitalRead(pins_[i]) == HIGH && reverse_ == true) return i;
				}
			}
			return ncPinIdx;
		}
		void resetState()
		{
			lastState_ = (lastState_==0)?-1:0;
		}
		void pollInput() {
			char state = readState();
			if (state != lastState_)
			{
				char buf[7];
				utoa(state, buf, 10);
				if (tryToSendDcsBiosMessage(msg_, buf))
					lastState_ = state;
			}
		}
	public:
		SwitchMultiPosT(const char* msg, const byte* pins, char numberOfPins, bool reverse = false) :
			PollingInput(pollIntervalMs),
			lastState_(0)
		{
			msg_ = msg;
			pins_ = pins;
			reverse_ = reverse;
			numberOfPins_ = numberOfPins;
			unsigned char i;
			for (i=0; i<numberOfPins; i++) {
				if( pins[i] != PIN_NC)
					pinMode(pins[i], INPUT_PULLUP);
			}
			lastState_ = readState();
		}
		
		void SetControl( const char* msg )
		{
			msg_ = msg;
		}
        
		void resetThisState()
		{
			this->resetState();
		}
	};
	typedef SwitchMultiPosT<> SwitchMultiPos;

in the public section you skip setting the pinMode for any PIN_NC which would seem to be correct.
then in the private section is where I beleive the issue to be

here is readState

		char readState() {
			unsigned char ncPinIdx = lastState_;
			for (unsigned char i=0; i<numberOfPins_; i++) {
				if( pins_[i] == PIN_NC)
					ncPinIdx = i;
				else
				{
					if (digitalRead(pins_[i]) == LOW && reverse_ == false) return i;
					else if (digitalRead(pins_[i]) == HIGH && reverse_ == true) return i;
				}
			}
			return ncPinIdx;
		}

you check if the pin is a PIN_NC and if so set ncPinIdx to the loop counter then return the the value of the loop count via return ncPinidx

now in Private pollInput()

		void pollInput() {
			char state = readState();
			if (state != lastState_)
			{
				char buf[7];
				utoa(state, buf, 10);
				if (tryToSendDcsBiosMessage(msg_, buf))
					lastState_ = state;
			}

you get the state which for the PIN_NC is going to be the ncPinidx value
since laststate was 2 and ncPinidx contatins 1 it will enter the if (state != lastState_) and then do the tryToSendDcsBiosMessage(msg_, buf) which we dont' want for a PIN_NC. we need it to not send a DcsBiosMessage in that case

one way around this would be to set ncPinidx to PIN_NC

		char readState() {
			unsigned char ncPinIdx = lastState_;
			for (unsigned char i=0; i<numberOfPins_; i++) {
				if( pins_[i] == PIN_NC)
					ncPinIdx = PIN_NC;
				else
				{
					if (digitalRead(pins_[i]) == LOW && reverse_ == false) return i;
					else if (digitalRead(pins_[i]) == HIGH && reverse_ == true) return i;
				}
			}
			return ncPinIdx;
		}

then in

		void pollInput() {
			char state = readState();
			if (state != lastState_)
			{
				char buf[7];
				utoa(state, buf, 10);
				if (tryToSendDcsBiosMessage(msg_, buf))
					lastState_ = state;
			}

check for PIN_NC and skip if that value is used

		void pollInput() {
			char state = readState();
			if (state != lastState_ || state != ncPinidx)
			{
				char buf[7];
				utoa(state, buf, 10);
				if (tryToSendDcsBiosMessage(msg_, buf))
					lastState_ = state;
			}

this way when state is PIN_NC the if loop is not entered so no DCS bios message is sent

[kbastronomics]

my logic may be off in
void pollInput() {
char state = readState();
if (state != lastState_ || state != ncPinidx)
{
char buf[7];
utoa(state, buf, 10);
if (tryToSendDcsBiosMessage(msg_, buf))
lastState_ = state;
}

the idea is that a non-connected pin should not result in a tryToSendDcsBiosMessage being sent

maybe this approach is better

		void pollInput() {
			char state = readState();
			if (state != PIN_NC) {  // do not send dcs a message if a non connected pin
				if (state != lastState_)
				{
					char buf[7];
					utoa(state, buf, 10);
					if (tryToSendDcsBiosMessage(msg_, buf))
						lastState_ = state;
				}
			 lastState_ = state;  // not sure this is needed
			}
		}

[kbastronomics]

looking thru history, I see why this was implemented this way as a default position for a multipostion switch
but it's had a side effect that when a non-shorting switch is used you get messages sent during the time the switch is between 2 positions which is not desirable.