1.1beta11 actions not working at all

[marius]

It sends zero emails. ON/OFF doesn't matter.

[marius]

Log entries

Logs:

020988:20060523:194305 In apply_actions(triggerid:14315,alarmid:231,trigger_ value:0)
020988:20060523:194305 Applying actions
020988:20060523:194305 SQL [select actionid,userid,delay,subject,message,recipient,ma xrepeats,repeatdelay,scripts,actiontype from actions where nextcheck<=1148438585 and status=0]
020988:20060523:194305 In check_action_conditions [actionid:1]
020988:20060523:194305 In check_action_condition [actionid:1,conditionid:19:cond.value:4]
020988:20060523:194305 Condition is FALSE
020988:20060523:194305 Conditions are FALSE
020988:20060523:194305 Conditions do not match our trigger. Do not apply actions.
020988:20060523:194305 Actions applied for trigger 14315 0

[marius]

found the culprit I think

actions.c line 530:

else if(condition->operator != CONDITION_OPERATOR_NOT_EQUAL)


should be changed to:

else if(condition->operator == CONDITION_OPERATOR_NOT_EQUAL)

Otherwise the next if statement (if(condition->operator == CONDITION_OPERATOR_MORE_EQUAL)) never gets executed.

So now i get the actions applied, but the status still never becomes OFF.

[marius]

On-&gt;off

fixed that too.

actions.c above line 702: I added trigger->value = trigger_value;

expression.c line 690: changed to if(trigger->value == TRIGGER_VALUE_FALSE)

I hope i didn't break anything else. But it seems to work, for now. What I don't get is that I don't receive the "OFF" msg unless I have "repeat" set to 2.

[Alexei]

Thanks for the patch. You're 100% correct.

actions.c line 530:

else if(condition->operator != CONDITION_OPERATOR_NOT_EQUAL)


should be changed to:

else if(condition->operator == CONDITION_OPERATOR_NOT_EQUAL)

Otherwise the next if statement (if(condition->operator == CONDITION_OPERATOR_MORE_EQUAL)) never gets executed.

So now i get the actions applied, but the status still never becomes OFF.

[Alexei]

I do not quite understand what have you changed. Please add more details.

fixed that too.

actions.c above line 702: I added trigger->value = trigger_value;

expression.c line 690: changed to if(trigger->value == TRIGGER_VALUE_FALSE)

I hope i didn't break anything else. But it seems to work, for now. What I don't get is that I don't receive the "OFF" msg unless I have "repeat" set to 2.

[marius]

I do not quite understand what have you changed. Please add more details.

I didn't spend a whole lot of time on the 2nd patch, but i think it's correct.

Basically substitute_macros in expression.c uses trigger->value to set the status "ON" or "OFF", but trigger->value was not getting set by apply_actions in actions.c before passing trigger pointer to substitute_macros.

So I set trigger->value using trigger_value (in fact, i'm confused why trigger_value is being used when there's trigger->value).

Also in substitute_macros, it evaluates trigger->value incorrectly when it sets the status to ON or OFF. So I had to swap the constant (TRIGGER_VALUE_TRUE -> TRIGGER_VALUE_FALSE).

[Alexei]

Basically substitute_macros in expression.c uses trigger->value to set the status "ON" or "OFF", but trigger->value was not getting set by apply_actions in actions.c before passing trigger pointer to substitute_macros.

So I set trigger->value using trigger_value (in fact, i'm confused why trigger_value is being used when there's trigger->value).

Also in substitute_macros, it evaluates trigger->value incorrectly when it sets the status to ON or OFF. So I had to swap the constant (TRIGGER_VALUE_TRUE -> TRIGGER_VALUE_FALSE).

trigger_value - new value of trigger
trigger->value - current one

If you look at substitute_macros() you'll see that:

else if( (s = strstr(str,"{STATUS}")) != NULL )
                {
                        /* This is old value */
                        if(trigger->value == TRIGGER_VALUE_TRUE)
                        {
                                snprintf(tmp,sizeof(tmp)-1,"OFF");
                        }
                        else
                        {
                                snprintf(tmp,sizeof(tmp)-1,"ON");
                        }

So, of old value is TRUE we assume than new value is FALSE and v.s, so everything is fine.

I'm seriously concerned about validity of your patch. Thanks for trying anyway

[marius]

trigger_value - new value of trigger
trigger->value - current one

If you look at substitute_macros() you'll see that:

else if( (s = strstr(str,"{STATUS}")) != NULL )
                {
                        /* This is old value */
                        if(trigger->value == TRIGGER_VALUE_TRUE)
                        {
                                snprintf(tmp,sizeof(tmp)-1,"OFF");
                        }
                        else
                        {
                                snprintf(tmp,sizeof(tmp)-1,"ON");
                        }

So, of old value is TRUE we assume than new value is FALSE and v.s, so everything is fine.

I'm seriously concerned about validity of your patch. Thanks for trying anyway

Ah that makes sense now. But if I revert back my patch the problem still remains for me. I never get the OFF messages, even when the status should be OFF. They're always on. Perhaps the current value never gets changed to the new value? I will keep looking some more tomorrow.

[Alexei]

I see both ON and OFF messages on my test system running the latest code. I'm doing more tests later today.

[marius]

I see both ON and OFF messages on my test system running the latest code. I'm doing more tests later today.

Ok apparently my testing methodology was incorrect. I kept changing the trigger definition to test the status change, which worked for beta9. When I tested by truly simulating real changes at the agent, it worked fine. thanks.