[Asterisk-code-review] res pjsip: Default endpoints to the "offline" status. (asterisk[13])

[Alexei Gradinari]

Alexei Gradinari has posted comments on this change.

Change subject: res_pjsip: Default endpoints to the "offline" status.
......................................................................


Patch Set 1: -Code-Review

> > > > I think if AST_DEVICE_UNKNOWN then AST_EXTENSION_UNAVAILABLE
 > > >
 > > > Unfortunately, this would break extension/device state
 > reporting
 > > > for lots of different channel types. For instance, Local
 > > channels,
 > > > since they do not technically exist when not in use, are
 > reported
 > > > as "unknown". In reality, the extension is not in use. I also
 > > think
 > > > this would cause problems for DAHDI analog channels, but I'm
 > not
 > > > 100% sure of that.
 > >
 > > Are sure about Local channel and state "unknown"?
 > > I think AST_DEVICE_INVALID is the default device state for Local
 > > channel.
 > >
 > > By the way if there isn't AOR associated with endpoint the state
 > > should be 'unknown'.
 > >
 > > if (ast_strlen_zero(persistent->aors)) {
 > > ast_endpoint_set_state(persistent->endpoint, AST_ENDPOINT_UNKNOWN);
 > > } else {
 > > ast_endpoint_set_state(persistent->endpoint, AST_ENDPOINT_OFFLINE);
 > > }
 > 
 > My Local channel knowledge is based on years-old experience when
 > doing a lot of app_queue development, so I very well could be wrong
 > about the way it works now.
 > 
 > Why should an endpoint with no AORs be unknown? If the Endpoint has
 > no AORs, then the endpoint cannot be reached. Should it not be
 > offline in that case as well?

For what reason are the states AST_ENDPOINT_UNKNOWN and AST_DEVICE_UNKNOWN needed?

I was worried that if set AST_ENDPOINT_OFFLINE, then it could cause the device state change publications for all endpoints, which could cause mass AMI/XMPP events.
I tested, and there wasn't any propagation.
So my review is neutral, because I still think that the correct state should be 'UNKNOWN'
for all objects: endpoint, device, extension.

-- 
To view, visit https://gerrit.asterisk.org/3674
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie99b84169393983453076f5e9c0d35ff313a456a
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: No