What is the proper way for me to handle network access for a device

user_8638882 0
Viewed 104

I am currently working on a blazor server project which will display information from modbus tcp/ip devices. I have a class called "DeviceModel" which models a Modbus device. A simplified example is shown below.

public string DeviceName {get;set;}
public string IpAddress {get;set;}
public string Port {get;set;}
public int[] Registers {get;set;}
public string Alarm1 {get;set;}

The device model class also contains methods to parse information from the Registers. For example the snippet below will check the value at a certain index in the Registers array. Based on that value it will set the Alarm1 property to ON or OFF.

public void CheckAlarm1(){
int status = Registers[4];
Alarm1 = status == 1 ? "ON" : "OFF";

}

I have another class called "NetworkAccess" which handles the TCP/IP connection to a device. A simplified example is shown below

// ModbusClient is a package which handles the reading/writing to TCP/IP Modbus 
private ModbusClient _client;
public string IPAddress {get;set;}
public string Port {get;set;}
public DeviceModel Device {get;set;}

public NetworkAccess(DeviceModel dev){
      IPAddress = dev.IPAddress;
      Port = dev.Port;
      _client = new ModbusClient(IPAddress,Port);
      _client.Connect();
}

The NetworkAccess class handles reading and writing data to/from the device on the network. An example method which would write data to a single register on the Modbus device is below.

public void WriteSingleRegister(int address,int dataToAdd){
     _client.WriteSingleRegister(address,dataToAdd);
}

Within my Razor Component for the webpage, within the OnInitialized() method I get a List containing DeviceModels from a database which fills in information such as IPAddress,Port, and Name for each device. To read information to the device, I have another method "GetData()" shown below

public async void GetData(){
     foreach(var device in Devices){
          NetworkAccess network = new NetworkAccess(dev);
          var dataUpdate = await network.ReadRegistersAsync(0,20);
          dev.Registers= dataUpdate;
     }
}

The way I currently have this setup works fine. In order to write to a device I would do something like this in my Razor Component

NetworkAccess network = new NetworkAccess(dev);
network.WriteRegistersAsync(0,new int[] {0,0,0,...}};

Where I am having trouble is I am not sure the of the correct (or best) way to handle my situation. In my head it makes more sense to me if I had methods within my DeviceModel class for specific operations such as "ResetAlarm1" or "ClearRegisters". That way I could do

dev.ResetAlarm1();

rather than doing this in my razor component below

NetworkAccess net = new NetworkAccess(dev)
dev.WriteRegister(6,0); // where 6 is the register to write to and 0 is value to write

I guess my question is should I add "NetworkAccess" to the device model and handle creating the connection and reading/writing to the device within that? Or does it make more sense to keep NetworkAccess and DeviceModel seperate?

I hope this post makes sense. This is more a question about design than it is about fixing a problem. While my current solution is working fine, I want to better understand if this is the correct approach or if I am way off.

Thanks for any help!

modbus blazor-server-side tcp oop c#
1 Answers

Or does it make more sense to keep NetworkAccess and DeviceModel separate?

As single responsibility principle of SOLID says:

The single-responsibility principle (SRP) is a computer-programming principle that states that every module, class or function in a computer program should have responsibility over a single part of that program's functionality, and it should encapsulate that part. All of that module, class or function's services should be narrowly aligned with that responsibility.

Read more about single responsibility principle of SOLID here.

So making separate method dev.ResetAlarm1() in Device class is more preferable for me.

It is hard to say whether my refactoring code is appropriate to you, but I tried to do my best:

public class Device
{
    public string Name { get; set; }

    public string IpAddress { get; set; }

    public string Port { get; set; }

    public int[] Registers { get; set; }

    public string[] Alarms { get; set; }


    public void CheckAlarm(int registerIndex)
    {
        int status = Registers[registerIndex];
        Alarms[registerIndex] = status == 1 ? "ON" : "OFF";
    }
}
user_16462400
Related