<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 1: Code-Review-1</p><p style="white-space: pre-wrap; word-wrap: break-word;">(34 comments)</p><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></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">They actually deleted the entire protocols module after 2.5.5. There's nothing to send upstream.<br></p><p><a href="https://gerrit.asterisk.org/c/testsuite/+/13222">View Change</a></p><p>20 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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">s/directle/directly</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Did you mean to leave this commented? No tests use it in their configs? If so the commented code can […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">whitespace</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">whitespace</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">more whitespace</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Can remove commented code?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Heh, no. I should have uncommented it. :)</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Commented code that can be removed? And 4 items of whitespace.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><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></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">The unit test isn't standalone. You need to create network activity for it to run and you need to examine the results. Maybe at a later date we can create a sipp scenario or a full blown test for it.</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">eol whitespace</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">looks like this can all be removed. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">whitespace</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">whitespace</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">whitespace</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">whitespace</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">whitespace</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">whitespace at eol</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">whitespace at eol</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">remove commented code.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">This can be deleted from the file vs commented.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Actually this test was already skipped. I commented it out to test it. It hasn't worked in a long time.</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This is pretty much the same as what's in pcap.py. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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: Tue, 19 Nov 2019 14:21:13 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>