<p style="white-space: pre-wrap; word-wrap: break-word;">Since you ported most things to contructs 2.9 would it make sense to submit those changes upstream to that project for inclusion there? Then we can eventually remove it from our code base.</p><p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222">View Change</a></p><p>34 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1//COMMIT_MSG">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1//COMMIT_MSG@30">Patch Set #1, Line 30:</a> <code style="font-family:monospace,monospace">directle</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">s/directle/directly</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py">File lib/python/asterisk/pcap.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@63">Patch Set #1, Line 63:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">#        if (module_config.get('register-observer')):<br># <br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Did you mean to leave this commented? No tests use it in their configs? If so the commented code can be deleted instead.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@69">Patch Set #1, Line 69:</a> <code style="font-family:monospace,monospace">        </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@119">Patch Set #1, Line 119:</a> <code style="font-family:monospace,monospace">       </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@166">Patch Set #1, Line 166:</a> <code style="font-family:monospace,monospace">        </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">more whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@734">Patch Set #1, Line 734:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">#        self.packet_factory.create_factory(RTPPacketFactory)<br>#        self.packet_factory.create_factory(RTCPPacketFactory)<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Can remove commented code?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@748">Patch Set #1, Line 748:</a> <code style="font-family:monospace,monospace">        </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Moar!</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@875">Patch Set #1, Line 875:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">#        if not 'register-observer' in module_config:<br>#            raise Exception('VOIPListener needs register-observer to be set')<br>        <br>        VOIPSniffer.__init__(self, module_config, test_object)<br>        <br>        packet_type = module_config.get("packet-type")<br>        bpf = module_config.get("bpf-filter")<br>        <br>        if packet_type:<br>            self.add_callback(packet_type, module_config.get("callback"))<br>                <br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Commented code that can be removed? And 4 items of whitespace.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@903">Patch Set #1, Line 903:</a> <code style="font-family:monospace,monospace"># This is a unit test for capture and parsing.</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This unit test should be moved into its own file and put under the lib/python/asterisk/self_test director</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@905">Patch Set #1, Line 905:</a> <code style="font-family:monospace,monospace"># UDP port 5060. </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">eol whitespace</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/test_case.py">File lib/python/asterisk/test_case.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/test_case.py@688">Patch Set #1, Line 688:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">#   It _appears_ no tests use this method but we'll throw an exception<br>#   to catch tests we might have missed.<br>    <br>#        raise NotImplementedError()<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">looks like this can all be removed. Including the whitespace :-)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/README.md">File lib/python/protocols/README.md:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/README.md@6">Patch Set #1, Line 6:</a> <code style="font-family:monospace,monospace">need that functionality. </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/README.md@10">Patch Set #1, Line 10:</a> <code style="font-family:monospace,monospace">module but was removed after v2.5.5.  Unfortunately, </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/README.md@12">Patch Set #1, Line 12:</a> <code style="font-family:monospace,monospace">module available.   </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/README.md@35">Patch Set #1, Line 35:</a> <code style="font-family:monospace,monospace"># PYTHONPATH=/usr/src/asterisk/testsuite/lib/python python3 </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/layer4/tcp.py">File lib/python/protocols/layer4/tcp.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/layer4/tcp.py@10">Patch Set #1, Line 10:</a> <code style="font-family:monospace,monospace">                                    </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/layer4/tcp.py@16">Patch Set #1, Line 16:</a> <code style="font-family:monospace,monospace">    "header_length" / ExprAdapter(Nibble, </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace at eol</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/layer4/udp.py">File lib/python/protocols/layer4/udp.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/layer4/udp.py@14">Patch Set #1, Line 14:</a> <code style="font-family:monospace,monospace">    "payload_length" / ExprAdapter(Int16ub, </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace at eol</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py">File lib/python/protocols/unconverted/application/dns.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py@108">Patch Set #1, Line 108:</a> <code style="font-family:monospace,monospace">    Rename("answers", </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py@122">Patch Set #1, Line 122:</a> <code style="font-family:monospace,monospace">    </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py@138">Patch Set #1, Line 138:</a> <code style="font-family:monospace,monospace">    </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py@140">Patch Set #1, Line 140:</a> <code style="font-family:monospace,monospace">    </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py@144">Patch Set #1, Line 144:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">    <br>    <br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/dhcpv4.py">File lib/python/protocols/unconverted/layer3/dhcpv4.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/dhcpv4.py@151">Patch Set #1, Line 151:</a> <code style="font-family:monospace,monospace">        Lanstar = 9,        </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/dhcpv4.py@194">Patch Set #1, Line 194:</a> <code style="font-family:monospace,monospace">    </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/dhcpv6.py">File lib/python/protocols/unconverted/layer3/dhcpv6.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/dhcpv6.py@48">Patch Set #1, Line 48:</a> <code style="font-family:monospace,monospace">        OPTION_CLIENT_FQDN = 39,        </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/icmpv4.py">File lib/python/protocols/unconverted/layer3/icmpv4.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/icmpv4.py@60">Patch Set #1, Line 60:</a> <code style="font-family:monospace,monospace">    Switch("code", lambda ctx: ctx.type, </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/icmpv4.py@67">Patch Set #1, Line 67:</a> <code style="font-family:monospace,monospace">    Switch("payload", lambda ctx: ctx.type, </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/icmpv4.py@72">Patch Set #1, Line 72:</a> <code style="font-family:monospace,monospace">        }, </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/icmpv4.py@84">Patch Set #1, Line 84:</a> <code style="font-family:monospace,monospace">    </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/igmpv2.py">File lib/python/protocols/unconverted/layer3/igmpv2.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/igmpv2.py@13">Patch Set #1, Line 13:</a> <code style="font-family:monospace,monospace">igmp_type = Enum(Byte("igmp_type"), </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/tests/channels/SIP/pcap_demo/run-test">File tests/channels/SIP/pcap_demo/run-test:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/tests/channels/SIP/pcap_demo/run-test@12">Patch Set #1, Line 12:</a> <code style="font-family:monospace,monospace">#try:</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">remove commented code.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/tests/channels/SIP/pcap_demo/test-config.yaml">File tests/channels/SIP/pcap_demo/test-config.yaml:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/tests/channels/SIP/pcap_demo/test-config.yaml@12">Patch Set #1, Line 12:</a> <code style="font-family:monospace,monospace">#    skip: True</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This can be deleted from the file vs commented.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/tests/channels/pjsip/rtp/rtp_keepalive/base/rtp.py">File tests/channels/pjsip/rtp/rtp_keepalive/base/rtp.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/13222/1/tests/channels/pjsip/rtp/rtp_keepalive/base/rtp.py@31">Patch Set #1, Line 31:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">        header = 'rtp_packet' / BitStruct(<br>                        'header' / Struct(<br>                                  'version' / BitsInteger(2),<br>                                  'padding' / Bit,<br>                                  'extension' / Bit,<br>                                  'csrc_count' / Nibble,<br>                                  'marker' / Bit,<br>                                  'payload' / BitsInteger(7)<br>                                  ),<br>                        'sequence_number' / Bytewise(Int16ub),<br>                        'timestamp' / Bytewise(Int32ub),<br>                        'ssrc' / Bytewise(Int32ub)<br>                        )<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is pretty much the same as what's in pcap.py. Can we just use the packet declaration from that file instead (would have to wrap with the 'rtp_packet')? Then all the contruct related code is self contained/referenced from only one place.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/testsuite/+/13222">change 13222</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/testsuite/+/13222"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: testsuite </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-Change-Id: Id38d01a2cd073b240fde909a38c95d69313bbbe7 </div>
<div style="display:none"> Gerrit-Change-Number: 13222 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 18 Nov 2019 21:26:22 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>