PepperDash / Essentials

The Essentials Application Framework Libraries
MIT License
133 stars 77 forks source link

Added new Interfaces and JoinMap for PDUs #947

Closed TrevorPayne closed 2 years ago

TrevorPayne commented 2 years ago

Resolves #183

Open for comment by @jonnyarndt @ngenovese11

TrevorPayne commented 2 years ago

Updates made

Requesting review from @ngenovese11 @jonnyarndt and @ediablo80

TrevorPayne commented 2 years ago

My only thought is that the IHasControlledPowerOutlets.PduOutletsCount might be redundant? Would it ever be different than the count on the PduOutlets collection?

Also PduOutlets is a dictionary that would expose publicly the ability to add/remove outlets via the dictionary add/remove methods. This should probably be a ReadOnlyDictionary.

Yeah - I could see this being problematic - I'll fix it.

TrevorPayne commented 2 years ago

Ok - so I'll remove the PDU Outlet Count, but you can't define an interface as readonly. I could potentially make it an abstract base class, but in that case, I would probably prefer to make it protected rather than readonly - I'd be worried about some instance where we want to query a device for what it has, rather than build out the Outlet list in the constructor.

ngenovese11 commented 2 years ago

Since the collection is a { get; } only property, the collection itself is already "readonly" as in you can never reassign the reference, but you can still add/remove from it even if you can't reassign the property with a setter. I would make the collection type a ReadOnlyDictionary, its a crestron provided collection