How to design hardware interfaces in C# while staying DRY and SOLID?

Question

I am having a design problem with an application that handles two or more networked I/O devices.

Both devices share properties like a name, IP address, and port. They will also share methods such as Connect(), Disconnect(), IsConnected().

In an effort to stay DRY, this leads me to believe I need some interface - IODevice.

public interface IODevice
{
    int Id { get; set; }
    string Name { get; set; }
    IPAddress IPAddress { get; set; }
    int Port { get; set; }
    
    bool IsConnected();
    void Connect();
    void Disconnect();
}

With both devices defined:

public class DeviceOne : IODevice
{

    private DeviceOneApi _api;
    
    public int Id { get; set; }
    public string Name { get; set; }
    public IPAddress IPAddress { get; set; }
    int Port { get; set; }
        
    public DeviceOne()
    {
        _api = new DeviceOneApi();
    }

    public bool IsConnected()
    {
        return _api.IsDeviceConnected();
    }

    public void Connect() 
    {
        _api.OpenConnection();
    }
    
    public void Disconnect()
    {
        _api.CloseConnection();
    }

}

public class DeviceTwo : IODevice
{
    ...
}

I will need to monitor the I/O to detect changes - this could be a digital sensor or a bit-change in memory. I was thinking of using some controller to loop through all defined devices and check the device I/O. An event aggregator will be used to send out notifications.

public class IOController
{
    private Thread _monitorThread;
    private int _monitorDelay;  
    private bool _canMonitor;
    private ILogger _logger;
    
    public static List<IODevice> Devices { get; set; }
    
    public IOController(ILogger logger)
    {
        _logger = logger;
        _monitorDelay = 100;
        _monitorThread = new Thread(Monitor);
    }
    
    public void AddDevice(IODevice device)
    {
        Devices.Add(device);
    }
    
    public void StartMonitor()
    {
        _canMonitor = true;
        _monitorThread.Start();
    }
    
    private void Monitor()
    {
        while(_canMonitor)
        {
            foreach(IODevice device in Devices)
            {
                // Check I/O Points
                EventAggregator.Instance.Publish(new SomeIOChange(IODetails));
                Thread.Sleep(_monitorDelay)
            }
        }
    }
}

This is the first place I am trying to make a decision. Each device implements its own types of I/O. I.e. DeviceOne may use integers in memory and DeviceTwo may use booleans from digital signals. A device may also use multiple types of I/O - digital, analog, strings, etc and the device API will implement methods to read/write each of these types.

So, I could either keep a seperate list of each device type and run multiple foreach loops:

foreach(IODeviceOne deviceOne in DeviceOnes) {  }
foreach(IODeviceTwo deviceTwo in DeviceTwos) {  }

Or, I could have the IODevice implement a method that checks its own I/O.

public interface IODevice
{
    ...
    void CheckIO();
}

private void Monitor()
{
    while(_canMonitor)
    {
        foreach(IODevice device in Devices)
        {
            device.CheckIO();
        }
    }
}

However, there will also be external scripts and user input that will need to read or modify an I/O type directly. With the IODevice interface defined in a way to be shared across devices, there is not a specific implementation of read/write.

For instance, a user may hit a toggle on the front-end that will affect a digital output in device one. Eventually, this action needs to be propagated to:

public class DeviceOne : IODevice
{
    ...
    public void WriteDeviceOnePointTypeOne(DeviceOnePointDetails details)
    {
        _api.WriteDeviceOnePointTypeOne(details);
    }
}

Since I am using EventAggregator, should I just implement the event listeners in each device instance?

public class DeviceOne : IODevice, ISubscriber<DeviceOnePointUpdate>
{
    ...
    
    public void OnEvent(DeviceOnePointTypeOneUpdate e)
    {
        _api.WriteDeviceOnePointTypeOne(e.Details)
    }
}

Or should I just use specific interfaces all the way down even though this may not follow DRY?

public DeviceOneController : IDeviceOneController
{
    ...
    private void Monitor()
    {
        while(_canMonitor)
        {
            foreach(IODeviceOne deviceOne in DeviceOnes)
            {
                // Check for I/O updates in device one
            }
        }
    }
}

Would casting be an option while still remaining SOLID?

public class FrontEndIOEventHandler
{
    private ILogger _logger;
    private IOController _controller;
    
    public FrontEndIOEventHandler(ILogger logger, IOController controller)
    {
        ...
    }

    public void UpdateDeviceOnePointTypeOne(int deviceId, DeviceOnePointTypeOneUpdate details)
    {
        DeviceOne deviceOne = _controller.GetDeviceById(deviceId) as DeviceOne;
        deviceOne.WriteDeviceOnePointTypeOne(details);
    }
}

I am using interfaces to aid in unit-testing and I eventually want to implement an IoC container. The device information will come from some external configuration and this configuration will include a device map (the only important I/O points will be defined by the user).

<devices>
    <device type="device_one">
        <name></name>
        <ip_address></ip_address>
        <port></port>
        <map>
            <modules>
                <module type="point_type_one">
                    <offset>0</module>
                    <points>
                        <point>
                            <offset>0</offset>
                        </point>
                        <point>
                            <offset>1</offset>
                        </point>
                    </points>
                </module>
            </modules>
        </map>
    </device>
    <device type="device_two">
        <name></name>
        <ip_address></ip_address>
        <port></port>
        <map>
            <integers>
                <integer length="32" type="point_type_four">
                    <offset>0</module>
                </integer>
            </integers>
        </map>
    </device>
</devices>

The overall goal is to read the configuration, instantiate each device, connect to them, start monitoring the specified I/O for changes, and have the device available for direct read/write.

I am open to all comments and criticisms! Please let me know if I am way off. I am still a novice and attempting to become a better (read: more professional) developer. I know there are ways to do this quick and dirty, but this is part of a bigger project and I'd like this code to be maintainable + extensible.

Let me know if I need to provide any additional detail! Thanks.

Answer 1

I tried to keep this code maintainable an extensible

public interface IIODevice
{
    int Id { get; set; }
    string Name { get; set; }
    IPAddress IPAddress { get; set; }
    int Port { get; set; }

    bool IsConnected();
    void Connect();
    void Disconnect();
    void CheckIO();
    void WriteDevicePointType(DevicePointTypeUpdateBase details);

}

devices defined

sealed class DeviceOne : IIODevice
{
    public int Id { get; set; }
    public string Name { get; set; }
    public IPAddress IPAddress { get; set; }
    public int Port { get; set; }

    private readonly IDeviceApi deviceApi;
    // Constructor injection of DeviceApi
    public DeviceOne(IDeviceApi deviceApi) => this.deviceApi = deviceApi;

    public void Connect()
    {
        deviceApi.OpenConnection();
    }

    public void Disconnect()
    {
        deviceApi.CloseConnection();
    }

    public bool IsConnected()
    {
        return deviceApi.IsDeviceConnected();
    }

    public void CheckIO()
    {
        Console.WriteLine("....");
    }

    public void WriteDevicePointType(DevicePointTypeUpdateBase details)
    {
        var pointType = details as DeviceOnePointTypeOneUpdate;
        deviceApi.WriteDevicePointType(pointType);
        Console.WriteLine(pointType);
    }
}

Add more device ...

sealed class DeviceTwo : IIODevice {}
sealed class DeviceThree : IIODevice {}

Device Api Interface (with this we don't need to modify your IController when new Device comes)

interface IDeviceApi
{
    bool IsDeviceConnected();
    void OpenConnection();
    void CloseConnection();

    void WriteDevicePointType(DevicePointTypeUpdateBase details);
}

Add Device Specific Api ...

sealed class DeviceOneApi : IDeviceApi { }
sealed class DeviceTwoApi : IDeviceApi { }
sealed class DeviceThreeApi : IDeviceApi { }

Creating an abstract for IOController (you can inject this to your FfronEndController and any others)

 public abstract class IOControllerBase
{
    private Thread _monitorThread;
    private int _monitorDelay;
    private bool _canMonitor;
    private ILogger _logger;

    public static List<IIODevice> Devices { get; set; }

    public IOControllerBase(ILogger logger)
    {
        _logger = logger;
        _monitorDelay = 100;
        _monitorThread = new Thread(Monitor);
        Devices = new List<IIODevice>();
    }

    public void AddDevice(IIODevice device)
    {
        Devices.Add(device);
    }

    public IIODevice GetDeviceById(int deviceID)
    {
        return Devices.FirstOrDefault(x => x.Id == deviceID);
    }

    public void StartMonitor()
    {
        _canMonitor = true;
        _monitorThread.Start();
    }

    protected void Monitor()
    {
        while (_canMonitor)
        {
            foreach (IIODevice device in Devices)
            {
                // Check I/O Points
                // If you have any parameter for each device 
                // you can create a ParameterBase abstract class
                // and inherit for each device which have different and common
                // check parameter.
                device.CheckIO();
                Thread.Sleep(_monitorDelay);
            }
        }
    }
}
public class IOController : IOControllerBase
{
    public IOController(ILogger logger) : base(logger) { }
}

/* For Unit Testing Purpose */
public class UnitTestIOController : IOControllerBase
{
    public UnitTestIOController(ILogger logger) : base(logger) { }
}

Your FrontEndIOEventHandler looks like

public class FrontEndIOEventHandler
{
    private ILogger _logger;
    private IOControllerBase _controller;
    public FrontEndIOEventHandler(ILogger logger, IOControllerBase controller)
    {
        _logger = logger;
        _controller = controller;
    }

    public void UpdateDevicePointTypeOne(int deviceId, DeviceOnePointTypeOneUpdate details) // or use DevicePointTypeUpdateBase
    {
        IIODevice deviceOne = _controller.GetDeviceById(deviceId);
        deviceOne.WriteDevicePointType(details);
    }
}

DevicePointTypeUpdateBase

public abstract class DevicePointTypeUpdateBase
{
    public int CommonPropery { get; set; }
}

DeviceOne Specific

public class DeviceOnePointTypeOneUpdate : DevicePointTypeUpdateBase
{
    public int DeviceOneIOValue { get; set; }
}

DeviceTwo Specific

public class DeviceTwoPointTypeOneUpdate : DevicePointTypeUpdateBase
{
    public bool DeviceOneIOValue { get; set; }
}

so all the components are loosely coupled, Use any IoC Container to Inject Dependency

You can close your class for editing , add extension method for adding any new feature in any of your component use Extension Methods to base componets. Adding more Device will not break you IOController ,ie.. DeviceFour. Whenever we adding DeviceFour you should create new DeviceFour concrete class and DeviceFourApi (if you can't use any Existing api) this will be easily attach with your IOController.

Happy Coding..