Re: Patch for new dissector plugin for Ether-S-Bus telegrams modified (Jaap Keuter)

Hi,

thank you for your feedback. I finally managed to modify my code by
	- using symbols instead of hardcoded command codes
	- removing the debug code
	- removing the last set in the patch 
	- adding the Makefile.common in the plugins dir

Could you please have a look at it and if it is ok, check it in?

Thanx,
chris

 

On Sun, 2006-06-04 at 12:00 -0500, [email protected]
wrote:
> Send Ethereal-dev mailing list submissions to
> 	[email protected]
> 
> To subscribe or unsubscribe via the World Wide Web, visit
> 	http://www.ethereal.com/mailman/listinfo/ethereal-dev
> or, via email, send a message with subject or body 'help' to
> 	[email protected]
> 
> You can reach the person managing the list at
> 	[email protected]
> 
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Ethereal-dev digest..."
> 
> 
> Reply-To: Ethereal development <[email protected]>
> 
> Today's Topics:
> 
>    1. Re: Patch for new dissector plugin for Ether-S-Bus	telegrams
>       (Jaap Keuter)
>    2. Re: ieee802.11 AID calculation patch (Joerg Mayer)
>    3. one more h245/per alignment issue (Cvetan Ivanov)
> 
> 
> ----------------------------------------------------------------------
> 
> Message: 1
> Date: Sun, 4 Jun 2006 13:32:17 +0200 (CEST)
> From: Jaap Keuter <[email protected]>
> Subject: Re: [Ethereal-dev] Patch for new dissector plugin for
> 	Ether-S-Bus	telegrams
> To: Ethereal development <[email protected]>
> Message-ID: <[email protected]>
> Content-Type: TEXT/PLAIN; charset=US-ASCII
> 
> Hi,
> 
> This looks pretty good, thanks.
> 
> A few small remark though:
> 1. could you use meaningfull symbols in the code i.s.o. hardcoded values.
> I'm refering to code like this:
> 
> +                     if (((request_val->cmd_code) == 0x00) ||
> +                            ((request_val->cmd_code) == 0x06) ||
> +                            ((request_val->cmd_code) == 0x07) ||
> +                            ((request_val->cmd_code) == 0x1E) ||
> +                            ((request_val->cmd_code) == 0x1F) ||
> +                            ((request_val->cmd_code) == 0xA1)){
> 
> 2. Could you clean up the code a bit by removing the debug code from it
> 
> 3. The last change in the set is superflous.
> 
> Thanx,
> Jaap
> 
> 
> 
> On Sat, 3 Jun 2006, Christian Durrer wrote:
> 
> > Hi,
> >
> > I hope this is the right place to send a patch for a new dissector
> > plugin. The telegram it dissects are for the communication between PLCs
> > (Programmable logic controller) from the company Saia Burgess Controls
> > Ltd. (http://www.start-controls.com, http://www.saia-burgess.com or
> > http://www.sbc-support.ch). The protocol is named Ether-S-Bus (the PLCs
> > are called PCD2, PCD3, PCD4).
> > We developped this dissector for easyer debugging of networks containing
> > our contollers.
> >
> > The patch was created against Ethereal Version 1.1.0 (SVN Rev 18189). I
> > tested it with various capture files (which some of them also fuzzed).
> > The VariousSBus is one of them.
> >
> > I also would like to make you a big compliment! Ethereal is a great
> > tool!
> >
> > I'm available for any your questions
> >
> > With best regards
> >
> > Christian Durrer
> >

> 
> End of Ethereal-dev Digest, Vol 38, Issue 7
> *******************************************

_______________________________________________
Ethereal-dev mailing list
[email protected]
http://www.ethereal.com/mailman/listinfo/ethereal-dev