Menu
▾
▴
RE: Re: [jModbus-devel] Codebase Merge initial comments
|
From: Proctor, K. <Kel...@al...> - 2001-10-25 00:05:38
|
Dieter,
I think I have solved the problem I put forward below. I'm proposing a
revised design for the ModbusASCIIInputStream class.
First a new exception class will need to be defined, the
EndOfFrameException.
The class will still extend the FilterInputStream class.
Internally the class would wrap the InputStream into a PushbackInputStream,
allowing it to peek ahead as required.
The class would look something like the following.
public class ModbusASCIIInputStream extends FilterInputStream {
private PuchbackInputStream m_Input;
private int m_BytesLeftInFrame;
private int m_LrcSum;
private byte[] m_Buffer;
public ModbusASCIIInputStream(InputStream in) {
m_Input = new PushbackInputStream(in, Modbus.MAX_MESSAGE_SIZE *
2);
m_Buffer = new byte[Modbus.MAX_MESSAGE_SIZE];
}
public int read() {
- Check if there is another byte (not delimeters or LRC)
available
- If available return next byte and update LrcSum.
- else throw EndOfFrame Exception
- update m_BytesLeftInFrame accordingly
}
public int read(byte[] b, int off, int len) {
- Check if there is at least another byte (not delimeters or
LRC) available.
- If available
- int byteToReturn = min(len,m_BytesLeftInFrame)
- Copy read byteToReturn bytes into m_Buffer using
m_Input.read(m_Buffer, 0, byteToReturn)
- Scan over m_Buffer to update m_LrcSum
- Copy bytes into b using System.arraycopy(m_Buffer, 0, b,
off, byteToReturn)
- return byteToReturn
- NOTE: Above avoids looped use of m_Input.read() (which is
byte banging input method and should be avoided if at all possible)
instead using the block read function and the
System.arraycopy
- Else (no more bytes available)
- throw EndOfFrameException
- update m_BytesLeftInFrame accordingly
}
public int read(byte[] b, int off, int len) {
return read(b, 0. b.length);
}
public void nextFrame() {
- read past the frame delimeters
- peek ahead to look for end of frame (using read then unread
for a large block of data)
- reset the m_BytesLeftInFrame variable
}
public int bytesLeftInFrame() {
return m_BytesLeftInFrame;
}
public boolean checkLRC() {
- If this is called when BytesLeftInFrame==0 then the LRC value
is checked and result returned
- If this is called at any other point then false is returned
(This is so if you think there should only been 10 bytes in
the frame and there are really
20 bytes then this will return false causing you to abort
the transaction)
}
}
I'm not 100% happy with the way the LRC check is performed above but I do
really like the way that we avoid byte banging the input stream using read()
all the time. It also extends FilterInputStream in a sensible way by making
all the read functions work properly.
What do you think of the above proposal and can you suggest any better
mechanism for checking the LRC? It would be nice of you could check the LRC
at the start somehow (before you start processing the message) as opposed to
at the end when you have wasted all this runtime processing a bogus message.
This becomes more important for write commands as you need to know if the
frame is OK before you do anything. With a read you can simply abort the
message at the last moment.
Hope you exams went (is going) well.
Cheers,
Kelvin
> -----Original Message-----
> From: Proctor, Kelvin [SMTP:Kel...@al...]
> Sent: Wednesday, October 24, 2001 11:10
> To: 'jModbus Devel'
> Subject: RE: Re: [jModbus-devel] Codebase Merge initial comments
>
> Dieter,
>
> Good luck with your exam. I hope this is not taking too much of your time
> away from such things.
>
> Comments below.
>
> Cheers,
>
> Kelvin
>
> > -----Original Message-----
> > From: Dieter Wimberger [SMTP:wi...@oe...]
> > Sent: Wednesday, October 24, 2001 5:07
> > To: jmo...@li...
> > Subject: RE: Re: [jModbus-devel] Codebase Merge initial comments
> >
> > Hi Kelvin,
> >
> > Well I have also to get back to some real work, as I am up to do an exam
> > tomorrow :)
> >
> > However, here some explanations on the questions you had:
> >
> >
> > 1. Why not reading frames from the ASCIIInputStream?
> >
> > Well this is easily answered, as this is the only way to make use of the
> > structure as it is in my package. The message implementations can keep
> > reading what is in their responsibility themselves, as the stream can be
>
> > passed on without problems.
> >
> > The Transport class has the responsibility of knowing frames, and
> handling
> > frames as such. This means you could simply pass on the : character and
> > the
> > CRLF character (anything outside 0-9 A-F) should do the job.
> > The transport should read from the stream, check for the start character
> > and the address
> > (if it is addressed, and we need to think about where this address comes
> > from :) etc....
> >
> [Proctor, Kelvin]
> I'm still note sure how you intend that this be done. Perhaps a
> little diagram below will illustrate the difficult I am having. It is how
> you 'pass on' the : and CR LF characters that I don't understand.
>
> <FixedWidthFont>
>
> Serial Port Stream (consider it to be 7 bit ASCII)
> ---------------------------------------------------
> | : | A | 2 | 3 | F | 9 | 4 | CR | LF | : | A | E |
> ---------------------------------------------------
>
> Translates to byte stream
> ---------------------------------------------------
> | ? | 0xA2 | 0x3F | 0x94 | ? | ? | 0xAE |
> ---------------------------------------------------
>
> </FixedWidthFont>
>
> I am unsure what byte values would go in the place of the ?
> characters in the bottom stream.
>
> > 2. Why producing the LRC byte for byte?
> >
> > Well this is not much of an issue, if you check out the sources of the
> I/O
> > streams of the
> > Java API, you will realize that at the end of the day all your reads end
> > up in
> > native public int read() somewhen.
> > Despite that your code will loop over the byte array too, so performance
> > of the
> > one byte at a time is probably even better then looping.
> > I am still not exactly sure however if the LRC is performed on each raw
> > byte (i.e. still encoded)
> > or on the decoded ones. I will check this out when I find time again,
> i.e.
> > after my exam.
> >
> [Proctor, Kelvin]
> I'm also unsure when you would propose checking the LRC if the frame
> is not extracted as a whole at the transport layer.
>
> There are a number of checks that need to be made that rely on
> knowing the length of the modbus message.
> 1. The LRC check can't occur until you find the end of the frame to
> do the check.
> 2. If a message states that it has 10 bytes to follow and there are
> infact 20 bytes following in the message then an exception should be
> generated as this is a malformed (even if not fatal) request.
>
> I can see a way that streams all the way could work for the output
> streams but I can't picture it for the input streams. I still see a need
> to
> at some point have the whole message sitting in a byte array (hidden in a
> class probably) so that a number of checks can be performed on it,
> including
> determining it length and the LRC.
>
> > 3. Why using a FilterInputStream?
> >
> > The use of FilterInputStream is for model reasons as it does exactly
> what
> > is stated also in the API,
> > it transforms the raw data exposing a standard interface.
> > All reads in Java are blocking, and I do not exactly understand what you
> > mean with "byte banging",
> > probably you can explain this term to me.
> > I guess I have to code this out to see what it does and whether it works
> > out :)
> >
> [Proctor, Kelvin]
> Byte banging is where you do things one byte at a time. This is
> often see with read and write operations. Block operations are usually of
> the order of 100 time more efficient then byte-by-byte operations. We are
> going to have to do that at one point at least (2 ASCII chars to 1 byte)
> but
> se should be going out of our way to minimise it if possible.
>
> > 4. Why the Modbus interface?
> >
> > This interface is solemnly serving the purpose of holding globally
> > accessible constants.
> > I have realized that it does not make any difference if you use a class
> or
> > an interface
> > with constants, despite that the class needs extra care to prevent
> > instantiation (i.e.
> > a private constructor). I am not even sure, but I think the compiler
> makes
> > the same out
> > of it (got to check the bytecode specs once to know).
> >
> > The use is Modbus.CONSTANT_NAME in any case from anywhere in the rest of
> > the code as it
> > is completely public.
> >
> [Proctor, Kelvin]
> This was really a non-issue, I just wanted to know if there was some
> reasoning and importance I had missed.
>
> > 6. Why the ModbusTransaction interface?
> >
> > This is one of the contracts I have been talking about.
> > I think it is reasonable to go like this, as for example a Transaction
> for
> > ModbusASCII transported
> > messages might have to be implemented differently.
> > The rest of the code using transactions can be coded against the
> > interface, and would not realize any
> > difference :)
> >
> [Proctor, Kelvin]
> I see now. I had perceived the wrong meaning of the word
> transaction. I had though this referred to a transaction up at function
> code level, this interface now make quite a lot of sense.
>
> One thing I have learnt over the last year is the importance of what
> names things are called. A slightly different name for the same object
> can
> give people a very different impression of what the object does
>
> > 7. Why the ModbusMessage interface?
> >
> > This is probably the only place where it is not so necessary to make
> such
> > a construct.
> > However, if you want to optimize or make a different implementation of
> > ModbusMessage then the one given by ModbusMessageImpl) you could go
> ahead
> > without the need to change certain other parts of your code (e.g the
> > transports for example). All this depends on how good your model and
> your
> > abstraction/specialization was :)
> >
> >
> > 8. What is a Singleton?
> >
> > Well this is a Pattern that serves the purpose of having only one
> instance
> > at runtime.
> > Basically there are different approaches to this in Java, however, the
> way
> > I implemented it,
> > is stemming from a very good book on object oriented programming/design
> > patterns.
> > This "bible" is
> > Gamma, Erich, Richard Helm, Ralph Johnson, and John Vlissides. 1995.
> > Design Patterns. Addison-Wesley. Reading,Mass.
> >
> > There are entries in the code snippets for Java, with some changes, even
> > done by people that are a bit strange, just overwriting parts and not
> even
> > documenting what they exactly did ;)
> >
> [Proctor, Kelvin]
> This is as I though, I had just never hear the term, before. Thanks
> for the information.
>
> > Regards,
> > Dieter
> >
> > _______________________________________________
> > jModbus-devel mailing list
> > jMo...@li...
> > https://lists.sourceforge.net/lists/listinfo/jmodbus-devel
>
> Alcoa World Alumina Australia is a trading name of Alcoa of Australia
> Limited, ACN 004 879 298
>
>
> _______________________________________________
> jModbus-devel mailing list
> jMo...@li...
> https://lists.sourceforge.net/lists/listinfo/jmodbus-devel
Alcoa World Alumina Australia is a trading name of Alcoa of Australia Limited, ACN 004 879 298
|
