Fix Wireshark dissector and add snapshot tests

[rotu]

Handle bit flags in Wireshark dissector (fix #815, fix #726)
Add tests for Wireshark (via tshark binary and pytest via the syrupy snapshot library)
Remove defunct mav2pcap.py script

[hamishwillee]

@rotu Tried this out. Certainly fixes the error we were seeing. I particularly like how clear the bitflags are rendered:

I also looked at many other message, including commands, and nothing seems broken:

Excellent to see test code of this. Would be good to expand it in future as a validation test. Out of scope for this probably.

The generator code changes make sense to me, and I think this also makes the generated code much more clear.

However I am not expert. Once you have this to a state you are happy with, we'll need to work with @peterbarker to get this in.

[rotu]

I'm not an expert either. Thanks for the screenshots! I think they highlight some other bugs!

The second message you show is a COMMAND_INT, where param2 is a float.

The value shown is 1065353216 which I think is supposed to be the floating point value 1.0f, corresponding to 1; i.e. MAV_DO_REPOSITION_FLAGS_CHANGE_MODE: True.

Also the longitude/latitude values are payload parameters 5/6 so I think they should be int32_t, not float, hence the wonky-looking values.

Could you please attach some pcap data I can test against?

[hamishwillee]

Looks like you're right. I ran this for a few seconds and then did a save-as. Is this what you needed?

pcapdata.zip

PS, more of an expert than me.

[hamishwillee]

PS Also a CI error

[rotu]

I see the CI errors and but the alternative is to use the obsolete pkg_resources and I hate writing new legacy code. When are we dropping python 2.7 and 3.6 support per #556?

[hamishwillee]

@rotu Could not agree more.
PX4 have dropped support for Python 2, and I understand that ArduPilot either have done so, or will be doing so very shortly. If those dependencies are gone, we should be able to switch over "very soon". I'll push for that, so ignore the error for now.
Wr.t. dropping Python 3.6 that is harder - we are in discussion.

As I understand it you're looking at the other issues with the generated output. Since I'm very happy for you to do so I'm going to just keep quiet now :-)

[hamishwillee]

@rotu This isn't waiting on me for anything is it?

[hamishwillee]

Hi @rotu
Looks like you are still progressing. Let me know when you need more review.

[rotu]

@hamishwillee Yep. I'm chipping away slowly. I'm a little hung up on getting the output to match when there are long lines (not sure how to get the output of tshark to wrap to a consistent line length)

[rotu]

Alright I'm sufficiently sick of this PR, so it's time to get it out there!

[hamishwillee]

@rotu I can imagine. Time for me to get a technical review on this?

[rotu]

@rotu I can imagine. Time for me to get a technical review on this?

I don't understand what you mean. Do whatever you need, loop in whoever you like, and ask whatever questions to satisfy your curiosity.

[hamishwillee]

@peterbarker Can you review this please. I think it is important to get in.

I have done basic tests on it by running the instructions in https://mavlink.io/en/guide/wireshark.html and also specifically looking at the HEARTBEAT message to check that the known issue was fixed.
This definitely fixes the original problem in a logic way, and the resulting output is also good.

@rotu Has also added a heap of test code. I have not tested that, since I have no merge rights, and you will have to do this anyway.

[peterbarker]

@rotu could you fix the commit list, please? Should be a single commit

[peterbarker]

... should have said I'm happy enough to merge this with a good commit list.

[rotu]

... should have said I'm happy enough to merge this with a good commit list.

@peterbarker
Feel free to squash merge. https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/configuring-commit-squashing-for-pull-requests

[peterbarker]

Merged, thanks!

[hamishwillee]

Hurray. Thanks!