Using while loops within data_events

PhreaK · October 2008

I've been warned about the use of while loops within the processing of data_events as it supposedly stops mainline execution whilst the loop is operating. Is this true, or am I being had?

Basically I've got a structure set up that I'm keeping device state information in. Occasionally the device will spit a string of commands at me and I need to be able to process them all in the most efficient way.

Currently I've gone down two routes:
In the first case I set up a while loop that iterates through DATA.TEXT removing and processing each command found. This seems to work a treat and is nice and neat, with minimal code.
The alternate option I've explored is passing the received data into a buffer, and processing one event per run off the mainline, if there's anything in the buffer. This is achieved by setting up a call to remove and process the first command currently in the buffer. Doing it this way also *works* (functionally speaking) however it introduces a good 3 second delay into the system.

My question is, is processing all data received (135bytes max length for this device) in one hit when it is received some horribly sacrilegious action in the world of AMX?

Spire_Jeff · October 2008

I use while loops occasionally when the situation fits. I know there are some programmers that avoid them like the plague, but I think this stems from experience with them in axcess. It could also be that some of them don't like the fact that they HAVE to make sure there is no chance the loop will never end. There is a very real possibility that you can lock up a processor if the while loop becomes infinite.

All that being said, I don't think there is any problem in using a while loop as you describe it.

I would be interested in seeing the code you are using for the mainline implementation tho. There shouldn't be a 3 second delay with the mainline version unless you are doing a lot of processing.

Jeff

PhreaK · October 2008

If I was doing a lot of processing I could understand it, however what I was asking it to do was extremely basic - unless I've got some really dumb logic in there, but I couldn't spot it. Anyway, here's the mainline processing version:

PROGRAM_NAME='Denon TU-604CI'
(* 
    DENON TU-604CI MULTIPLE TUNER
    -----------------------------
    Programmer: Kim Burgess
    Notes:
	Requires straight-through RS232 cable.
	Requires 'Option Setup > RS-232C Power' set to "ON" on the device.
	Once defined in the DEV array TUNERS[], each device can be controlled by passing their array
	index to the methods. Alternatively all devices in the system can be send a command by passing
	tuner_num 0.
	
    Communication format:
	9600bps,8,N,1,1
	135 bytes (max) data length
	
    Methods:
	TU_POWER(tuner_num,action)	 	Change or query the power state of the whole unit
	TU_SLOT_POWER(tuner_num,slot,action)	Change or query the power state of a specific slot
*)

DEFINE_CONSTANT
// Power commands
POFF=0
PON=1
STAT=100
(***********************************************************)
DEFINE_TYPE
STRUCTURE ANALOG_INFO
{
    INTEGER FREQUENCY
    INTEGER TUNER_BAND
    INTEGER MODE
    CHAR NAME[8]		// RDS PS
    CHAR PROGRAM_TYPE[8]	// RDS PTY
}
STRUCTURE DAB_INFO
{
    INTEGER CHAN
    INTEGER MODE
    CHAR NAME[8]
    CHAR LONG_NAME[16]
    CHAR ENSEMBLE_NAME[16]
    CHAR PROGRAM_TYPE[16]
    INTEGER FREQUENCY
    INTEGER MONO_STEREO
    INTEGER SIGNAL
}
STRUCTURE SLOT_INFO
{
    INTEGER POWER
    INTEGER TYPE
    INTEGER PRESET
    ANALOG_INFO ANALOG_STATE
    DAB_INFO DAB_STATE
}
STRUCTURE DENON_INFO
{
    CHAR RESPONSE_BUFF[255]
    INTEGER POWER
    INTEGER SOURCE
    SLOT_INFO SLOT_STATE[6]
}
(***********************************************************)
DEFINE_VARIABLE
DEV TUNERS[]={TUNER}
DENON_INFO TUNER_STATE[LENGTH_ARRAY(TUNERS)]
VOLATILE INTEGER TUNER_NUM
VOLATILE INTEGER SLOT_NUM
VOLATILE INTEGER COUNT
(***********************************************************)
DEFINE_CALL 'TU_POWER'(INTEGER TUNER_NUM, INTEGER ACTION)
STACK_VAR CHAR DV_COMMAND[7]
{
    SWITCH(ACTION)
    {
	CASE PON:{DV_COMMAND='ON'}
	CASE POFF:{DV_COMMAND='STANDBY'}
	CASE STAT:{DV_COMMAND='?'}
    }
    IF(TUNER_NUM>0)
	SEND_STRING TUNERS[TUNER_NUM],"'PW',DV_COMMAND,$0D"
    ELSE
	SEND_STRING TUNERS,"'PW',DV_COMMAND,$0D"
}
DEFINE_CALL 'TU_SLOT_POWER'(INTEGER TUNER_NUM, INTEGER SLOT_NUM, INTEGER ACTION)
STACK_VAR CHAR DV_COMMAND[7]
{
    SWITCH(ACTION)
    {
	CASE PON:{DV_COMMAND='ON'}
	CASE POFF:{DV_COMMAND='STANDBY'}
	CASE STAT:{DV_COMMAND='?'}
    }
    IF(TUNER_NUM>0)
	SEND_STRING TUNERS[TUNER_NUM],"'PWS',ITOA(SLOT_NUM),DV_COMMAND,$0D"
    ELSE
	SEND_STRING TUNERS,"'PWS',ITOA(SLOT_NUM),DV_COMMAND,$0D"
}
DEFINE_CALL 'PROCESS_RESPONSE'(INTEGER TUNER_NUM)
LOCAL_VAR CHAR RESPONSE[32]
{
    RESPONSE=REMOVE_STRING(TUNER_STATE[TUNER_NUM].RESPONSE_BUFF,"$0D",1)
    SELECT
    {
	ACTIVE(FIND_STRING(RESPONSE,'PWON',1)):{TUNER_STATE[TUNER_NUM].POWER=PON}
	ACTIVE(FIND_STRING(RESPONSE,'PWSTANDBY',1)):{TUNER_STATE[TUNER_NUM].POWER=POFF}
	ACTIVE(MID_STRING(RESPONSE,3,1)='S'):	// String was a slot state return
	{
	    SLOT_NUM=ATOI(MID_STRING(RESPONSE,4,1))	// Grab the slot number from the received string
	    SELECT
	    {
		ACTIVE(FIND_STRING(RESPONSE,'PW',1)):	// Slot power states
		{
		    SELECT
		    {
			ACTIVE(FIND_STRING(RESPONSE,'ON',5)):{TUNER_STATE[TUNER_NUM].SLOT_STATE[SLOT_NUM].POWER=PON}
			ACTIVE(FIND_STRING(RESPONSE,'STANDBY',5)):{TUNER_STATE[TUNER_NUM].SLOT_STATE[SLOT_NUM].POWER=POFF}
		    }
		}
	    }
	}
    }
}
(***********************************************************)
DEFINE_START

(***********************************************************)
DEFINE_EVENT
DATA_EVENT[TUNERS]
{
    ONLINE:	// Initialize coms and grab the state of the unit
    {
	TUNER_NUM=GET_LAST(TUNERS)
	SEND_COMMAND TUNERS[TUNER_NUM], 'SET BAUD 9600 N,8,1 485 DISABLE'
	CALL 'TU_POWER'(TUNER_NUM,STAT)
	CALL 'TU_SLOT_POWER'(TUNER_NUM,1,STAT)
    }
    STRING:	// Pass the received data into the units processing cue
    {
	TUNER_NUM=GET_LAST(TUNERS)
	TUNER_STATE[TUNER_NUM].RESPONSE_BUFF="TUNER_STATE[TUNER_NUM].RESPONSE_BUFF,DATA.TEXT"
    }
}
(***********************************************************)
DEFINE_PROGRAM
FOR(COUNT=1;COUNT<=LENGTH_ARRAY(TUNERS);COUNT++)	// Process any pending repsonse data for each unit in the system
{
    IF(LENGTH_STRING(TUNER_STATE[COUNT].RESPONSE_BUFF)>0)
	CALL 'PROCESS_RESPONSE'(COUNT)
}

My mainline in my master axs then is just doing a couple of button updates to my panel.

a_riot42 · October 2008

I think your caps lock key is stuck on

I don't write code like that because I don't want a loop to continuously run in mainline when 99.99% of the time the buffer will be empty. Why not call your function directly from the string event once you have a full command string?
Paul

Btw, you can use invariants to make sure your while loop always terminates.

PhreaK · October 2008

a_riot42 wrote: »

I think your caps lock key is stuck on

I don't write code like that because I don't want a loop to continuously run in mainline when 99.99% of the time the buffer will be empty. Why not call your function directly from the string event once you have a full command string?
Paul

I know, I'd rather use CamelCase, but all caps is the standard for the place I'm working.

Calling the function from the string event is not possible as multiple command can be passed in on the one string, and as such I'd have to use a while loop within that function to loops through each command, which is essentially what I'm doing at the moment, but with the loop within data_event.

DEFINE_EVENT
DATA_EVENT[TUNERS]
{
    ONLINE:	// Initialize coms and grab the state of the unit
    {
	TUNER_NUM=GET_LAST(TUNERS)
	SEND_COMMAND TUNERS[TUNER_NUM], 'SET BAUD 9600 N,8,1 485 DISABLE'
	CALL 'TU_POWER'(TUNER_NUM,STAT)
	CALL 'TU_SLOT_POWER'(TUNER_NUM,ALL,STAT)
	CALL 'TU_SELECT_SOURCE'(TUNER_NUM,STAT)
    }
    STRING:	// Process the received data to update the units state info
    {
	TUNER_NUM=GET_LAST(TUNERS)
	WHILE(FIND_STRING(DATA.TEXT,"$0D",1))	// Occasionally multiple commands are recieved so make sure we break them up and process them all
	{
	    RESPONSE=REMOVE_STRING(DATA.TEXT,"$0D",1)			// Grab the front most received command
	    RESPONSE=LEFT_STRING(RESPONSE,LENGTH_STRING(RESPONSE)-1)	// Remove the trailing carridge return
	    SLOT_NUM=ATOI(MID_STRING(RESPONSE,4,1))			// Grab the slot number from the received string
	    SWITCH(LEFT_STRING(RESPONSE,2))
	    {
		CASE 'PW':	// Power states
		{
		    SELECT
		    {
			ACTIVE(RESPONSE='PWON'):{TUNER_STATE[TUNER_NUM].POWER=PON}
			ACTIVE(RESPONSE='PWSTANDBY'):{TUNER_STATE[TUNER_NUM].POWER=POFF}
			ACTIVE(FIND_STRING(RESPONSE,'ON',5)):{TUNER_STATE[TUNER_NUM].SLOT_STATE[SLOT_NUM].POWER=PON}
			ACTIVE(FIND_STRING(RESPONSE,'STANDBY',5)):{TUNER_STATE[TUNER_NUM].SLOT_STATE[SLOT_NUM].POWER=POFF}
		    }
		}
		CASE 'SI':	// Source select
		{
		    TUNER_STATE[TUNER_NUM].SOURCE=ATOI(RIGHT_STRING(RESPONSE,1))
		}
		CASE 'TU':	// Tuner type
		{
		    SWITCH(RIGHT_STRING(RESPONSE,2))
		    {
			CASE 'AN':{TUNER_STATE[TUNER_NUM].SLOT_STATE[SLOT_NUM].TYPE=AN}
			CASE 'XM':{TUNER_STATE[TUNER_NUM].SLOT_STATE[SLOT_NUM].TYPE=XM}
			CASE 'HD':{TUNER_STATE[TUNER_NUM].SLOT_STATE[SLOT_NUM].TYPE=HD}
			CASE 'DA':{TUNER_STATE[TUNER_NUM].SLOT_STATE[SLOT_NUM].TYPE=DA}
			CASE 'NO':{TUNER_STATE[TUNER_NUM].SLOT_STATE[SLOT_NUM].TYPE=NO}
		    }
		}
	    }
	}
    }
}

vining · October 2008

I like while loops but if you wanted to do something freakie you could have the string event trigger a timeline and have the timeline run the code like it it does in define_program (it might be a little slower) and then on a lenght_string check of all your buffers and if all lengths are 0 you can terminate the timeline from with in the timeline itself.

kbeattyAMX · October 2008

Why not parse from one buffer instead of multiple buffers? Just append to the information from data.text with the tuner number the string came from. Then there is only one buffer to track length. Then it should be easy to apply recursion with out using a FOR or WHILE loops. BTW, I like your application of structures! I try to avoid any looping when it comes to parsing data. And yes recursion is a type of looping. If done right its easily controlled.

kbeattyAMX · October 2008

Here's another thought... Why are you handling multiple tuners in one module? Why not one tuner per module and let module instancing handle all of the buffer separation?

Jeff · October 2008

Your loop looks perfect, I can't honestly think of any reason why it would be generating a 3 second gap.

The first thing I'd do is test to see how many times your loop is actually running. Create an integer variable, and immediately before the loop starts, set it to 0. Then make the first line of your while loop nInteger++. Watch your integer in debug and see what happens. If it only increments a little bit, then your loop is probably fine.

If that shows some strange variation, the next thing I would try is declaring a stack_var and setting it equal to data.text, then using it for the duration of the loop. Sometimes data.text does strange things, especially when you start removing characters from it.

You might also try putting the following line in your define_program section

wait 5 send_string 0,"'Heartbeat'"

If define_program is running correctly, that will output the line "Heartbeat" to diagnostics every half a second. You can watch that during the while loop to see if it is affected at all.

J

kbeattyAMX · October 2008

You could also find out how many time Mainline is running /second.

if (debug)
{
  MLcount++
  wait 10
  {
    send_string 0,"'HEARTBEAT>>>',itoa(MLcount)"
    MLcount = 0
  }
}

This would give you an ideal of how taxing your code is to the processor.

Jeff · October 2008

whats this if(debug) statement you used? I've never seen that before, how does it work?

kbeattyAMX · October 2008

this is there to keep this section of code running if you accidentally forgot to delete it. To run it, just go into debug in NS2 and change the value of debug to 1. When you reboot the value will go back to 0 if debug was not defined as persistent.

Jeff · October 2008

Ahh. yea, I do that already for my TP Feedback, to keep it from running during debugging. I was hoping there was a super secret undocumented way to have it automatically tell if I was debugging or not

a_riot42 · November 2008

PhreaK wrote: »

Currently I've gone down two routes:
In the first case I set up a while loop that iterates through DATA.TEXT removing and processing each command found. This seems to work a treat and is nice and neat, with minimal code.
The alternate option I've explored is passing the received data into a buffer, and processing one event per run off the mainline, if there's anything in the buffer. This is achieved by setting up a call to remove and process the first command currently in the buffer. Doing it this way also *works* (functionally speaking) however it introduces a good 3 second delay into the system.

I think you answered your own question, no?
Paul

PhreaK · November 2008

Cheers for the help guys. The way I'm handling it now is to keep the individual tuner's buffers within their state structure and running a timeline to process it. This way I can set the data_event to check if the timeline is active, and if not get one going. The timeline event will also collapse the timeline if there is no data in the buffer to process.

@kbeattyAMX
It's not actually a module, this is just an include as the place I'm working at doesn't like modules due to their 'lack of expandability'. If I had my way I'd be using them as they are there for a (good) reason, however I'm the new guy so I'm just trying to keep to the already established coding styles as it makes things easier for everyone

DHawthorne · November 2008

I've said it before, I'll say it again: we need an Idle() function. While loops don't actually halt your program, but they don't give up time slices either, the processor is dedicated to running them until they exit. If your loop only takes a millisecond to run, it won't matter, but big processes will bog down everything.

Chip Moody · November 2008

OMG. I never new there was a semi-official term for that. I'm blown away. (Maybe I shouldn't be?)

Thanks for cluing me in to that one, PhreaK...

- Chip

PhreaK wrote: »

I know, I'd rather use CamelCase, but all caps is the standard for the place I'm working.

Jeff · November 2008

I don't know about the rest of y'all, but I prefer writing in Hungarian Notation, which is more of a lowerCamelCase than UpperCamelCase. y'know, nInteger, sSinteger, cChar, cString, etc.

mpullin · November 2008

alternate way of dealing with unreasonable standards

PhreaK wrote: »

I know, I'd rather use CamelCase, but all caps is the standard for the place I'm working.

Maybe you should just write the code the way you want to write it and install an all-caps font on your boss's computers.

http://www.dafont.com/barrelhouse-all-cap.font

ericmedley · November 2008

I quite often write in all caps in my code. My eyesight's so bad it helps me at times.
otherwise, I don't $:\$

a_riot42 · November 2008

ericmedley wrote: »

I quite often write in all caps in my code. My eyesight's so bad it helps me at times.
otherwise, I don't $:\$

You can increase the size of the font if your eyesight is not great. Using all caps is harder to read due to the lack of word shape clues.
Paul

Using while loops within data_events

Comments