Rif.: numeric knob & code that must run outside event switch

[ElectroLund]

I have an interesting situation.  I have a control knob wherein I've enabled control via the user's mouse wheel (thanks to great sample code throughout these fora!). For this to work as I want it, I had to put the processing portion of the code outside the control's event switch, such that the value of the control's output would get updated immediately.

The only problem with this methodology is that then when the user closes the panel, the knob control gets one last callback, and that block of code gets executed one last time, which results in a "invalid control ID" non-fatal error.  The solution here is relevant, but not ideal as noted above.

My solution feels like a hack, but tell me what you think -- trap on EVENT_DISCARD and return early.  Seems to work, just feels like a patch.  Here's the code:

int CVICALLBACK KnobCallback(int panel, int control, int event, void *callbackData, int eventData1, int eventData2)
{
	int prevValue = 0;
	int currValue;			// current value of knob control, range = 0-100
	double currMotor = 0;
	double currMeter = 0;
	double idealMotor = 0;
	double currVoltage;
	int max, min, inc;

	// find the range values set in the UIR control:
	GetCtrlAttribute(panel, control, ATTR_MIN_VALUE, &min);
	GetCtrlAttribute(panel, control, ATTR_MAX_VALUE, &max);
	GetCtrlAttribute(panel, control, ATTR_INCR_VALUE, &inc);
	
	// load the current control changed value:
	GetCtrlVal(panel, control, &currValue);

	switch (event)
	{
		case EVENT_COMMIT:	// any control commit:
			break;
		
		case EVENT_MOUSE_WHEEL_SCROLL:

			switch (eventData1)
			{
				case MOUSE_WHEEL_SCROLL_UP:
					if (currValue < max)
						currValue += inc;  // increment 1 step at a time, not eventData2 number of steps (Windows scroll wheel number)
					else
					{
						currValue = max;	// hold at max
						return 1;	// Swallow event to prevent from updating UIR
					}
					break;

				case MOUSE_WHEEL_SCROLL_DOWN:
					if (currValue > min)
						currValue -= inc;  // decrement 1 step at a time, not eventData2 number of steps (Windows scroll wheel number)
					else
					{
						currValue = min;	// hold at min
						return 1;	// Swallow event to prevent from updating UIR
					}
					break;

				case MOUSE_WHEEL_PAGE_UP:
					if (currValue < max)
						currValue += (inc * 5);  // eventData2 = 0 when PAGE up/down
					else
					{
						currValue = max;	// hold at max
						return 1;	// Swallow event to prevent from updating UIR
					}
					break;

				case MOUSE_WHEEL_PAGE_DOWN:
					if (currValue > min)
						currValue -= (inc * 5);  // eventData2 = 0 when PAGE up/down
					else
					{
						currValue = min;	// hold at min
						return 1;	// Swallow event to prevent from updating UIR
					}
					break;
			}
			SetCtrlVal(panel, control, currValue);	// update control with processed value
			break;
			
		case EVENT_VAL_CHANGED:
			if ((currValue < prevValue) && (currValue > min))			// decrementing above floor
			{
				currValue -= inc;
			}
			else if ((currValue > prevValue) && (currValue < max))		// incrementing below ceiling
			{
				currValue += inc;
			}
			else if (currValue = max) // TODO: this condition doesn't work as expected; control doesn't trap for wrap-around from max to min, vice versa
			{
				currValue = max;	// hold at max
				return 1;	// Swallow event to prevent from updating UIR
			}
			else if (currValue = min) // TODO: this condition doesn't work as expected; control doesn't trap for wrap-around from max to min, vice versa
			{
				currValue = min;	// hold at min
				return 1;	// Swallow event to prevent from updating UIR
			}
			SetCtrlVal(panel, control, currValue);	// update control with processed value
			prevValue = currValue;	// update state variable
			break;
		
		case EVENT_DISCARD:
			return 0;	// TODO: bug fix for quitting cleanly, so that the code outside of the event switch doesn't execute one last time when the panel is quit.
			break;
			
	}	// end switch


	currVoltage =  (currValue * MOTOR_VOLT_STEP) + MOTOR_VOLT_MIN;
	currMotor = LabJackTimer(LABJACK_TIMER0);
	idealMotor = MOTOR_SLOPE * currVoltage;
	
	if (abs((int)(currMotor - idealMotor)) < MOTOR_TOL)
	{
		SetCtrlVal(panel, MAINPANEL_TEXTMSG6, "GOOD");
		SetCtrlAttribute(panel, MAINPANEL_TEXTMSG6, ATTR_TEXT_BGCOLOR, VAL_GREEN);
	}
	else 
	{
		SetCtrlVal(panel,MAINPANEL_TEXTMSG6,"FAIL");
		SetCtrlAttribute(panel, MAINPANEL_TEXTMSG6, ATTR_TEXT_BGCOLOR, VAL_RED);
	}
	return 0;
}

[Wolfgang · Accepted solution]

I would suggest placing the SetCtrlVal code fragment into a separate function and call this function for the correct event only; right now, it is called for any event including the discard event...

Also, you do not need to recall the min/max/inc values of your control every time, once on program startup should be enough

[ElectroLund]

Good points, Wolfgang.  Will investigate...

[RobertoBozzolo]

In my opinion your solution is not a "trick" but rather a legitimate coding: EVENT_DISCARD is a valid event you must consider while designing the control callback.

Moreover, as far as I can see you should add some more cases, or better differently trim your callback, since the code outside the switch is executed on a series of events you are not considering at present but your control is indeed receiveing: mouse clicks, keyboard events, EVENT_MOUSE_POINTER_MOVE (when the user moves the mouse over the control without actually interacting with it), GOT_ and LOST_FOCUS...

Probably the best thing is to put a default: return 0; case in the switch instead of the EVENT_DISCARD case.

[ElectroLund]

Roberto, you are very correct.  I was not realizing this was happening.  I forgot how beautiful the default case is!