7
\$\begingroup\$

Needed to get a local serial port list. System.IO.Ports.SerialPort.GetPortNames() returns names but not the descriptions like you can see in Device Manager: COM6 vs. USB Serial Port (COM6)

There is also no way to subscribe to plug-and-play port list changes.

Here comes the solution. Repository at GitHub.

To consume the lib:

static void Main(string[] args)
{
    using (var ports = new ComPortList())
    {
        WriteLine(Join("\n", ports));
        ports.Subscribe(WriteLine);                
        ReadLine();
    }
}  

Sample output on my machine is:

USB Serial Port (COM6)
USB Serial Port (COM6) was removed.
USB Serial Port (COM6) was added.

Where library code is:

public class ComPortList : 
    Enumerable<ComPort>, IObservable<ComPortUpdate>, IDisposable
{
    public ComPortList()
    {
        Watcher = new PnPWatcher();
        Items = new List<ComPort>();
        Subject = new Subject<ComPortUpdate>();
        Watcher.Change += (s, e) => Sync();
        Sync();
    }

    PnPWatcher Watcher { get; }
    List<ComPort> Items { get; }
    Subject<ComPortUpdate> Subject { get; }

    public void Dispose()
    {
        Watcher.Dispose();
        Subject.Dispose();
    }

    public override IEnumerator<ComPort> GetEnumerator()
    {
        lock (Items)
            return Items.ToList().GetEnumerator();
    }

    public IDisposable Subscribe(IObserver<ComPortUpdate> observer) =>
        Subject.Subscribe(observer);

    void Sync()
    {
        lock (Items)
        {
            var items = ComPort.Detect();

            foreach (var port in Items.Except(items).ToArray())
            {                    
                Items.Remove(port);
                Subject.OnNext(new ComPortRemoval(port));
            }

            foreach (var port in items.Except(Items).ToArray())
            {
                Items.Add(port);
                Subject.OnNext(new ComPortAddition(port));
            }
        }
    }
}

Using:

public class ComPort : ValueObject<ComPort>
{
    public static ComPort[] Detect()
    {
        using (var searcher = new ManagementObjectSearcher
            ("SELECT * FROM Win32_PnPEntity WHERE ClassGuid='{4d36e978-e325-11ce-bfc1-08002be10318}'"))
        {
            return searcher.Get().Cast<ManagementBaseObject>()
                .Select(p => new ComPort($"{p["Caption"]}"))
                .ToArray();
        }
    }

    ComPort(string description)
    {
        Description = description;
        Name = new string(
            description
                .Reverse()
                .SkipWhile(c => c != ')')
                .TakeWhile(c => c != '(')
                .Reverse()
                .ToArray())
            .TrimStart('(')
            .TrimEnd(')');
    }

    public string Description { get; }
    public string Name { get; }

    public override string ToString() => Description;
    protected override IEnumerable<object> EqualityCheckAttributes =>
        new object[] { Name, Description };
}

And:

class PnPWatcher : IDisposable
{
    const string Query =
        "SELECT * FROM Win32_DeviceChangeEvent";

    public PnPWatcher()
    {
        Watcher = new ManagementEventWatcher(Query);
        Watcher.EventArrived += (s, e) => Change(this, EventArgs.Empty);
        Watcher.Start();
    }

    ManagementEventWatcher Watcher { get; }
    SynchronizationContext Context { get; }

    public void Dispose() => Watcher.Dispose();
    public event EventHandler Change = delegate { };
}

Event classes are:

public abstract class ComPortUpdate
{
    public ComPortUpdate(ComPort port) => Port = port;
    public ComPort Port { get; }
}

public class ComPortAddition : ComPortUpdate
{
    public ComPortAddition(ComPort port) : base(port) { }
    public override string ToString() => $"{Port} was added.";
}

public class ComPortRemoval : ComPortUpdate
{
    public ComPortRemoval(ComPort port) : base(port) { }
    public override string ToString() => $"{Port} was removed.";
}

Utility classes are:

public abstract class Enumerable<T> : IEnumerable<T>
{
    public abstract IEnumerator<T> GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator() =>
        GetEnumerator();
}

And:

public abstract class ValueObject<T> : IEquatable<ValueObject<T>>
            where T : ValueObject<T>
{
    protected abstract IEnumerable<object> EqualityCheckAttributes { get; }

    public override int GetHashCode() =>
        EqualityCheckAttributes
            .Aggregate(0, (hash, a) => hash = hash * 31 + (a?.GetHashCode() ?? 0));

    public override bool Equals(object obj) =>
        Equals(obj as ValueObject<T>);

    public virtual bool Equals(ValueObject<T> other) =>
        other != null &&
        GetType() == other.GetType() &&
        EqualityCheckAttributes.SequenceEqual(other.EqualityCheckAttributes);

    public static bool operator ==(ValueObject<T> left, ValueObject<T> right) =>
        Equals(left, right);

    public static bool operator !=(ValueObject<T> left, ValueObject<T> right) =>
        !Equals(left, right);
}
\$\endgroup\$
  • 3
    \$\begingroup\$ ComPortList not implementing IList - hadn't expected this from you who recommended me the book on framwork design :-P It should be an ObservableComPortCollection or at least ComPortCollection. Also why does it have the List<ComPort> Items { get; } property if it's already a list? Are you sure this is your code? ;-) \$\endgroup\$ – t3chb0t Jun 20 '18 at 7:14
  • 1
    \$\begingroup\$ @t3chb0t the last snippet is his signature in my opinion :P \$\endgroup\$ – Denis Jun 20 '18 at 13:19
  • \$\begingroup\$ @t3chb0t 1) ComPortList sounds more natural - one would not say Collection about COM ports. From that book: "For example, a type used in mainline scenarios to submit print jobs to print queues should be named Printer, rather than PrintQueue. Even though technically the type represents a print queue and not the physical device (printer), from the scenario point of view, Printer is the ideal name because most people are interested in submitting print jobs and not in other operations related to the physical printer device (e.g., configuring the printer)." 2) ReadOnlyCollection<T>.Items? \$\endgroup\$ – Dmitry Nogin Jun 20 '18 at 13:33
2
\$\begingroup\$

PnPWatcher.Context is never used

In ComPortList:

    void Sync()
    {
        lock (Items)
        {
            var items = ComPort.Detect();

            foreach (var port in Items.Except(items).ToArray())
            {                    
                Items.Remove(port);
                Subject.OnNext(new ComPortRemoval(port));
            }

            foreach (var port in items.Except(Items).ToArray())
            {
                Items.Add(port);
                Subject.OnNext(new ComPortAddition(port));
            }
        }
    }

You are enumerating the list plenty of times with those .ToArray() If update the list of items after rising events you safe a lot of resources and is more comprensible

    void Sync()
    {
        lock (Items)
        {
            var currentItems = ComPort.Detect();

            foreach (var port in Items.Except(currentItems))
            {                    
                Subject.OnNext(new ComPortRemoval(port));
            }

            foreach (var port in currentItems.Except(Items))
            {
                Subject.OnNext(new ComPortAddition(port));
            }

            Items.Clear();
            Items.AddRange(currentItems);
        }
    }

ValueObject What is the purpose of this class ?

    public override int GetHashCode() =>
        EqualityCheckAttributes
            .Aggregate(0, (hash, a) => hash = hash * 31 + (a?.GetHashCode() ?? 0));

In this form "hash =" is useless and you are also risking an overflow exception

    public override int GetHashCode() =>
        EqualityCheckAttributes
            .Aggregate(0, (hash, a) =>
            {
                unchecked
                {
                    return hash * 31 + (a?.GetHashCode() ?? 0);
                }
            });

Enumerable:

public abstract class Enumerable<T> : IEnumerable<T>
{
    public abstract IEnumerator<T> GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator() =>
        GetEnumerator();
}

This class doesn't have any purpose, you can move the only not abstract method in the only class that inherits Enumerable (and remove confusion with System.Enumerable)

PS: If you want to simplify the interaction with WMI i wrote a wrapper around it:

https://www.nuget.org/packages/WindowsMonitor.Standard/

So to retrieves those ports:

var ports = PnPEntity.Retrieve()
                           .Where(x => x.ClassGuid == "{4d36e978-e325-11ce-bfc1-08002be10318}")
                            .ToArray();
\$\endgroup\$
  • 1
    \$\begingroup\$ In original code subscriber can access Items from OnNext event handler and get updated enumeration. Your Sync implementation breaks this behavior. \$\endgroup\$ – Nikita B Jun 20 '18 at 11:42
  • \$\begingroup\$ @NikitaB I don't know about OnNext() implementation but this code is inside a lock, nothing should be updated before it has been released. \$\endgroup\$ – Adriano Repetti Jun 20 '18 at 12:46
  • \$\begingroup\$ @AdrianoRepetti OnNext is Rx's equivalent of event invocation. So it can invoke an arbitrary delegate, including, for example: comPortList.Subscribe(cpu => comPortList.ToArray()). This delagete is called from inside the lock whenever OnNext is called. lock does not prevent reentrency. \$\endgroup\$ – Nikita B Jun 20 '18 at 13:05
  • \$\begingroup\$ What OP can do is store all changes in single event argument (similar to NotifyCollectionChangedEventArgs) and invoke event only once. Then suggested refactoring will work. \$\endgroup\$ – Nikita B Jun 20 '18 at 13:12
  • \$\begingroup\$ @NikitaB right! I was that blindly focused on ToArray() that I didn't even see the Remove()/Add() thing. :man_facepalming: \$\endgroup\$ – Adriano Repetti Jun 20 '18 at 13:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.