[Asterisk-code-review] lib/python/asterisk/pcap/py Replace construct_legacy.protocols (testsuite[13])
Kevin Harwell
asteriskteam at digium.com
Mon Nov 18 15:26:22 CST 2019
Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/testsuite/+/13222 )
Change subject: lib/python/asterisk/pcap/py Replace construct_legacy.protocols
......................................................................
Patch Set 1: Code-Review-1
(34 comments)
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.
https://gerrit.asterisk.org/c/testsuite/+/13222/1//COMMIT_MSG
Commit Message:
https://gerrit.asterisk.org/c/testsuite/+/13222/1//COMMIT_MSG@30
PS1, Line 30: directle
s/directle/directly
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py
File lib/python/asterisk/pcap.py:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@63
PS1, Line 63: # if (module_config.get('register-observer')):
: #
Did you mean to leave this commented? No tests use it in their configs? If so the commented code can be deleted instead.
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@69
PS1, Line 69:
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@119
PS1, Line 119:
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@166
PS1, Line 166:
more whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@734
PS1, Line 734: # self.packet_factory.create_factory(RTPPacketFactory)
: # self.packet_factory.create_factory(RTCPPacketFactory)
Can remove commented code?
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@748
PS1, Line 748:
Moar!
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@875
PS1, Line 875: # if not 'register-observer' in module_config:
: # raise Exception('VOIPListener needs register-observer to be set')
:
: VOIPSniffer.__init__(self, module_config, test_object)
:
: packet_type = module_config.get("packet-type")
: bpf = module_config.get("bpf-filter")
:
: if packet_type:
: self.add_callback(packet_type, module_config.get("callback"))
:
Commented code that can be removed? And 4 items of whitespace.
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@903
PS1, Line 903: # This is a unit test for capture and parsing.
This unit test should be moved into its own file and put under the lib/python/asterisk/self_test director
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/pcap.py@905
PS1, Line 905: # UDP port 5060.
eol whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/test_case.py
File lib/python/asterisk/test_case.py:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/asterisk/test_case.py@688
PS1, Line 688: # It _appears_ no tests use this method but we'll throw an exception
: # to catch tests we might have missed.
:
: # raise NotImplementedError()
looks like this can all be removed. Including the whitespace :-)
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/README.md
File lib/python/protocols/README.md:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/README.md@6
PS1, Line 6: need that functionality.
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/README.md@10
PS1, Line 10: module but was removed after v2.5.5. Unfortunately,
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/README.md@12
PS1, Line 12: module available.
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/README.md@35
PS1, Line 35: # PYTHONPATH=/usr/src/asterisk/testsuite/lib/python python3
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/layer4/tcp.py
File lib/python/protocols/layer4/tcp.py:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/layer4/tcp.py@10
PS1, Line 10:
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/layer4/tcp.py@16
PS1, Line 16: "header_length" / ExprAdapter(Nibble,
whitespace at eol
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/layer4/udp.py
File lib/python/protocols/layer4/udp.py:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/layer4/udp.py@14
PS1, Line 14: "payload_length" / ExprAdapter(Int16ub,
whitespace at eol
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py
File lib/python/protocols/unconverted/application/dns.py:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py@108
PS1, Line 108: Rename("answers",
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py@122
PS1, Line 122:
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py@138
PS1, Line 138:
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py@140
PS1, Line 140:
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/application/dns.py@144
PS1, Line 144:
:
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/dhcpv4.py
File lib/python/protocols/unconverted/layer3/dhcpv4.py:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/dhcpv4.py@151
PS1, Line 151: Lanstar = 9,
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/dhcpv4.py@194
PS1, Line 194:
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/dhcpv6.py
File lib/python/protocols/unconverted/layer3/dhcpv6.py:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/dhcpv6.py@48
PS1, Line 48: OPTION_CLIENT_FQDN = 39,
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/icmpv4.py
File lib/python/protocols/unconverted/layer3/icmpv4.py:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/icmpv4.py@60
PS1, Line 60: Switch("code", lambda ctx: ctx.type,
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/icmpv4.py@67
PS1, Line 67: Switch("payload", lambda ctx: ctx.type,
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/icmpv4.py@72
PS1, Line 72: },
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/icmpv4.py@84
PS1, Line 84:
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/igmpv2.py
File lib/python/protocols/unconverted/layer3/igmpv2.py:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/lib/python/protocols/unconverted/layer3/igmpv2.py@13
PS1, Line 13: igmp_type = Enum(Byte("igmp_type"),
whitespace
https://gerrit.asterisk.org/c/testsuite/+/13222/1/tests/channels/SIP/pcap_demo/run-test
File tests/channels/SIP/pcap_demo/run-test:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/tests/channels/SIP/pcap_demo/run-test@12
PS1, Line 12: #try:
remove commented code.
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:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/tests/channels/SIP/pcap_demo/test-config.yaml@12
PS1, Line 12: # skip: True
This can be deleted from the file vs commented.
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:
https://gerrit.asterisk.org/c/testsuite/+/13222/1/tests/channels/pjsip/rtp/rtp_keepalive/base/rtp.py@31
PS1, Line 31: header = 'rtp_packet' / BitStruct(
: 'header' / Struct(
: 'version' / BitsInteger(2),
: 'padding' / Bit,
: 'extension' / Bit,
: 'csrc_count' / Nibble,
: 'marker' / Bit,
: 'payload' / BitsInteger(7)
: ),
: 'sequence_number' / Bytewise(Int16ub),
: 'timestamp' / Bytewise(Int32ub),
: 'ssrc' / Bytewise(Int32ub)
: )
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.
--
To view, visit https://gerrit.asterisk.org/c/testsuite/+/13222
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: testsuite
Gerrit-Branch: 13
Gerrit-Change-Id: Id38d01a2cd073b240fde909a38c95d69313bbbe7
Gerrit-Change-Number: 13222
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Mon, 18 Nov 2019 21:26:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191118/8992b394/attachment-0001.html>
More information about the asterisk-code-review
mailing list