----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3490/#review11786 -----------------------------------------------------------
Ship it! Ship It! - Matt Jordan On April 29, 2014, 9:20 a.m., Mark Michelson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3490/ > ----------------------------------------------------------- > > (Updated April 29, 2014, 9:20 a.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-23672 > https://issues.asterisk.org/jira/browse/ASTERISK-23672 > > > Repository: testsuite > > > Description > ------- > > In ASTERISK-23672, it was reported that when the presence state of an entity > was changed such that the state was the same but the subtype or message > differed from what was previously set, NOTIFYs were not sent to SIP > subscribers. Looking at the code, it appeared that res_pjsip_exten_state was > being overzealous in trying to filter out repeated state changes and that the > core PBX code already performed the necessary filtering. > > I made the following change to res_pjsip_exten_state.c, which I'm not placing > in its own review due to its small size: > > Index: res/res_pjsip_exten_state.c > =================================================================== > --- res/res_pjsip_exten_state.c (revision 412578) > +++ res/res_pjsip_exten_state.c (working copy) > @@ -334,11 +334,6 @@ > struct notify_task_data *task_data; > struct exten_state_subscription *exten_state_sub = data; > > - if (exten_state_sub->last_exten_state == info->exten_state && > - exten_state_sub->last_presence_state == info->presence_state) { > - return 0; > - } > - > if (!(task_data = alloc_notify_task_data(exten, exten_state_sub, > info))) { > return -1; > } > > I then created three testsuite tests to ensure that behavior is as we expect > for it to be: > > devstate_repeat: This relies on the device state for a custom device state to > be "not in use" initially. A subscriber subscribes to a custom device state. > We then set a device state change for the custom device state to be "not in > use". Since there is no change in the device state, no NOTIFY should be sent > to the subscriber. > presencestate_repeat: This sets an initial presence state for a > CustomPresence entity. A SIP subscriber subscribes to the custom presence > state. We then set the presence state to the exact same value again and > ensure that no additional NOTIFYs are sent to the subscriber. > presencestate_repeat_okay: This sets an initial presence state for a > CustomPresence entity. A SIP subscriber subscribes to the custom presence > state. We then change the presence state twice. The first change keeps the > same state and message and changes the subtype. The second change keeps the > same state and subtype and changes the message. These should result in two > additional NOTIFYs being sent to the subscriber. > > > Diffs > ----- > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/tests.yaml 5004 > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/test-config.yaml > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/sipp/subscribe.xml > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/repeat_presence_state.py > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/configs/ast1/pjsip.conf > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat_okay/configs/ast1/extensions.conf > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/test-config.yaml > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/sipp/subscribe.xml > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/repeat_presence_state.py > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/pjsip.conf > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/extensions.conf > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/test-config.yaml > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/sipp/subscribe.xml > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/repeat_device_state.py > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/configs/ast1/pjsip.conf > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/devstate_repeat/configs/ast1/extensions.conf > PRE-CREATION > > Diff: https://reviewboard.asterisk.org/r/3490/diff/ > > > Testing > ------- > > Prior to the diff mentioned in the description, devstate_repeat and > presencestate_repeat would pass, but presencestate_repeat_okay would not. > With the diff above applied, all three tests pass. > > > Thanks, > > Mark Michelson > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
