Home AMX User Forum AMX General Discussion

Using while loops within data_events

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?

Comments

  • Spire_JeffSpire_Jeff Posts: 1,917
    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
  • PhreaKPhreaK Posts: 966
    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_riot42a_riot42 Posts: 1,624
    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.
  • PhreaKPhreaK Posts: 966
    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}
    		    }
    		}
    	    }
    	}
        }
    }
    
  • viningvining Posts: 4,368
    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.
  • 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.
  • 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?
  • JeffJeff Posts: 374
    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
  • 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.
  • JeffJeff Posts: 374
    whats this if(debug) statement you used? I've never seen that before, how does it work?
  • 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.
  • JeffJeff Posts: 374
    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_riot42a_riot42 Posts: 1,624
    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
  • PhreaKPhreaK Posts: 966
    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 :)
  • DHawthorneDHawthorne Posts: 4,584
    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.
  • 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.
  • JeffJeff Posts: 374
    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.

    :)
  • mpullinmpullin Posts: 949
    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
  • ericmedleyericmedley Posts: 4,177
    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_riot42a_riot42 Posts: 1,624
    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
Sign In or Register to comment.