Apollo3zehn / FluentModbus

Lightweight and fast client and server implementation of the Modbus protocol (TCP/RTU).
MIT License
189 stars 71 forks source link

The desire to add AOP functionality #64

Open Zoneee opened 2 years ago

Zoneee commented 2 years ago

Hello author, I am an Asian with poor English, all the following content is from machine translation. I am new to Modbus and would like the author to add AOP functionality, which would be useful to check if the commands are correct. Or please tell me how to add AOP, I think I can do the job.

Apollo3zehn commented 2 years ago

Hi Zoneee, could you please shortly descibe what AOP is? Do you have a link where it is documented?

Apollo3zehn commented 2 years ago

What dies AOP stand for? I cannot find anything Ehen I search for "AOP Modbus". Thanks :-)

Zoneee commented 2 years ago

Hi,Apollo3zehn,For AOP please refer to:

I use AOP more for logging. As for server.RegistersChanged&server.CoilsChanged

Zoneee commented 2 years ago

Hi,Apollo3zehn,For AOP please refer to:

I use AOP more for logging. As for server.RegistersChanged&server.CoilsChanged

Oh, I forgot to mention that it is the "Modbus Client" that requires AOP functionality. Or maybe there already is? What did I miss?

Apollo3zehn commented 2 years ago

I am new to AOP but from what I have read here (https://docs.microsoft.com/en-us/archive/msdn-magazine/2014/february/aspect-oriented-programming-aspect-oriented-programming-with-the-realproxy-class) you can implement it yourself without the need to change Fluentmodbus - except that I think you need an interface class, like an IModbusTcpClient. If that is really all you need I can add these interfaces.

Zoneee commented 2 years ago

Hello, SORRY for the late reply. I read the article carefully, and the "aspect-oriented Programming with the RealProxy Class" approach is a good one, but it's a little too complicated. Inheriting the RealProxy approach results in adding a new RealProxy that either changes the factory or the consumer knows which RealProxy is really needed. I didn't understand how to solve the problem by adding IModbusTcpClient. I think I can write a simple example of what I need, right

Zoneee commented 2 years ago

As shown in the following table, this is my understanding of the message format sent by the client and server of Modbus-RTU. I want to log in the same format as the table below

Sending and receiving data examples

<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns="http://www.w3.org/TR/REC-html40">

  | pcb | command | address | data | CRC -- | -- | -- | -- | -- | -- length | 1byte | 1byte | 2bytes | 2bytes | 2bytes example | 01 | 06 | 00 | 01 | 00 | 32 | A8 | 3D

Zoneee commented 2 years ago

Hello. Here's the simple example

Apollo3zehn commented 2 years ago

As far as I understand from your example you want to be able to modify and/or log the outgoing an incoming frames.

So you need access to the frame buffer right before https://github.com/Apollo3zehn/FluentModbus/blob/74024b81cd957f64140c24855e339b7d0226dfab/src/FluentModbus/Client/ModbusRtuClient.cs#L183

and right after https://github.com/Apollo3zehn/FluentModbus/blob/74024b81cd957f64140c24855e339b7d0226dfab/src/FluentModbus/Client/ModbusRtuClient.cs#L219

Is my understanding correct?

Why do you need the RequestHandler and the RequestEventHandler? Is one of them not enough to accomplish both? Or do you need a real event to allow more than one subscriptions to it?

Since the sent data is not available as string but as byte array, my idea would be to add a RequestHandler with a signature ike this:

public Func<Span<byte>> RequestHandler { get; set; }

You would then be able to use it like:

private static void ModifyMessage(Span<byte> message)
{
    message[5] = 0x45;
}

Would this suit your needs?

Alternatively, the signature could be extended to include the unit identifier and the function code which could make processing the data easier:

public Func<byte, ModbusFunctionCode, Span<byte>> RequestHandler { get; set; }

If you have a good reason for the RequestEventHandler I can add it as well.

Apollo3zehn commented 2 years ago

Okay I forgot one thing. When you modify the buffer, it will possibly have a new length. Therefore we would need something like this in the end:

public Func<byte, ModbusFunctionCode, byte[], int, int> RequestHandler { get; set; }

private static int ModifyMessage(byte unitIdentifier, ModbusFunctionCode functionCode, byte[] message, int messageLength)
{
    if (unitIdentifier == 1 && functionCode == ModbusFunctionCode.WriteMultipleRegisters)
    {
        message[5] = 0x45;
    }

    return messageLength;
}
Zoneee commented 2 years ago

hello
First of all, I'm very sorry. I think the AOP features proposed in this issue are poorly considered.

Before I proposed the issue, I wanted to add events to ModbusClient to provide logging. Based on this thought, I thought "why not add AOP to provide more functionality?" So I put forward this issue.

But when I finished the Demo I noticed that adding RequestHandler to ModbusClient to implement AOP was too intrusive, especially when you said "when you modify the buffer, it may have a new length." In order to keep AOP and Client Class responsibilities single, the combination pattern used by the previously rejected "RealProxy" is the more correct approach.

The idea is that you can use AOP or any other method to modify the message to be sent before you get into ModbusClient, but the message to be sent after you get into ModbusClient should not be changed.

So I want to close this issue and I will implement my idea on a new branch soon.

Something like this would provide notification of sent and received messages by adding events in ModbusClient and broadcasting them in a TransceiveFrameAsync. The implementation of AOP is left to developers who really need it. :) Thank you for your answer.

Zoneee commented 2 years ago

Oh, I forgot to answer your question.

Q: "Why do you need the RequestHandler and the RequestEventHandler? Is one of them not enough to accomplish both? Or do you need a real event to allow more than one subscriptions to it?" A: I think they have different responsibilities. The RequestHandler is used to implement AOP, while the RequestEventHandler is used to notify that a message has been sent. The RequestHandler has the ability to modify the message, while the RequestEventHandler just knows what happened and can't modify it.

Q: "If you have a good reason for the RequestEventHandler I can add it as well." A: As I said in my last answer, I'm more focused on Log than AOP. I think I can, but MAYBE I'll ask you some more questions when I get into difficulties.

A few digressions "Alternatively, the signature could be extended to include the unit identifier and the function code which could make processing the Data easier:" I agree. I use string only because I'm lazy.😊

Apollo3zehn commented 2 years ago

Thanks for your thoughts and comments. Do I understand it right that you will implement your ideas in a separate branch and when you are happy you will open another PR?

Zoneee commented 2 years ago

Yes, I have been preparing for the postgraduate exam, and the development work may be late

Zoneee commented 2 years ago

I have a question. In the modbus-RTU example, "client.connect ("COM1"); ", how can I be sure it is currently "COM1" and not something else? https://apollo3zehn.github.io/FluentModbus/samples/modbus_rtu.html

Apollo3zehn commented 2 years ago

You can't. Either you know the COM port beforehand or you need to loop through all of them and check for valid responses.