Skip to content

Feat/fix alicat multidrop #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
senslab-MarkHawkins wants to merge 5 commits into
base: Develop
Choose a base branch
from
Open

Feat/fix alicat multidrop #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cfa02e6
Quick fix for multidrop alicat serial connection
garath001 Jul 30, 2026
518da29
comments on mfcflowcontrollerconnection
garath001 Jul 30, 2026
fb65d85
Added units to parser
garath001 Aug 10, 2026
75b65a2
Unified setpoint units and maxvalue units
garath001 Aug 11, 2026
4e3827a
removed lines added for debugging
garath001 Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions AlicatBusMFC/AlicatBusMFC.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>net10.0</TargetFramework>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>

</Project>
13 changes: 12 additions & 1 deletion AlicatMFCRemastered/Connection/MassFlowControllerConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,18 @@ namespace AlicatMFCRemastered;

public class MassFlowControllerConnection : AresHardwareConnection, IMfcConnection
{
private readonly List<char> _unusedIds;
//simple registry to manage instances of seriaport resources across multiple mfcs
static Dictionary<string, MassFlowControllerConnection> _connections = new Dictionary<string, MassFlowControllerConnection>();

@nkleiner nkleiner Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if you could elaborate on what you wanted to implement this to address. Typically we don't expect the driver to care about it's connection in relation to any others (maintaining the idea that the driver itself would only really care about it's own connection). It's true that the MFC's can share a serial connection, and we account for that in ARES by allowing devices of the same class (i.e. Alicat MFC's) to share the same resource with a unique identifier. For the MFC's this is the Id list that use a single capital letter (A, B, C...). Unless there's something critical this provides, I would say this is best left at the ARES level rather than the driver level itself.

public static MassFlowControllerConnection GetMassFlowControllerConnection(string portName)
{
if (!_connections.ContainsKey(portName))
_connections[portName] = new MassFlowControllerConnection(portName);

return _connections[portName];
}


private readonly List<char> _unusedIds;

public MassFlowControllerConnection(string portName) : base(new SerialPortConnectionInfo(19200, Parity.None, 8, StopBits.One), portName,
new SerialConnectionOptions
Expand Down
36 changes: 23 additions & 13 deletions AlicatMFCRemastered/MassFlowController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace AlicatMFCRemastered;

public class MassFlowController : AresDevice, IMassFlowController
{
private readonly int _expectedDataFormatEntryCount;
private readonly int _expectedDataFormatEntryCount;
private readonly BehaviorSubject<AresStruct> _stateSubject = new(new AresStruct());
private CancellationTokenSource _stateGetterLoopTokenSource = new();
private CompositeDisposable _stateWatchers = new();
Expand Down Expand Up @@ -57,12 +57,13 @@ public MassFlowController(DeviceConnectionInfo connectionInfo, ILogger logger) :
}

else
_serialConnection = new MassFlowControllerConnection(serialInfo.PortName);
_serialConnection = MassFlowControllerConnection.GetMassFlowControllerConnection(serialInfo.PortName);

_stateWatchers = new CompositeDisposable

@nkleiner nkleiner Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the motivation here, but this should theoretically not be an issue on the current system. When receiving data that needs to be parsed, we have dedicated response parser classes that handle breaking down the serialized data. In the case of this stream, we're only going to get this processed response (the "LiveDataResponse" class) in the event that parser deemed it a valid response to parse. You can find that logic under "Commands/Responses/Parsers/LiveDataParser.cs". Based on that, I don't think this change is necessary, you shouldn't see the system processing any additional data from MFC's other than the one you told the driver to care about. If you are, then perhaps there's another bug happening under the hood we can dig out.

{
_serialConnection.GetTransactionStream<LiveDataResponse>().Select(transaction => transaction.Response).Subscribe(UpdateLiveData)
};
/// replaced with transactional update. Could restore if we add an id check to verify correct MFC, but given potential 26 MFCs on a single connection, most traffic on bus could be ignored
//_stateWatchers = new CompositeDisposable
//{
// _serialConnection.GetTransactionStream<LiveDataResponse>().Select(transaction => transaction.Response).Subscribe(UpdateLiveData)
//};

_expectedDataFormatEntryCount = _mfcType == MfcTypeEnum.Normal ? 12 : 7;

Expand Down Expand Up @@ -170,9 +171,12 @@ private void UpdatePotentialMaxValue(ManufacturerInfoEntry entry)
_logger.LogWarning($"Failed to get max value for MFC {Name} as we couldn't get the numeric max value from model number {entry.Data}");
return;
}
var flowVal = StandardVolumeFlow.From(numericNum, unit);
dataFrameFormat.MaxVal = flowVal.StandardLitersPerMinute.ToString();
}
_logger.LogInformation($"Found a potential max value of {numericNum} {unit} for MFC {Name} from model number {entry.Data}");
var flowVal = StandardVolumeFlow.From(numericNum, unit);
// must be converted to match setpoint units, otherwise may cause issues when calculating newsetpoint
// dataFrameFormat.MaxVal = flowVal.StandardLitersPerMinute.ToString();
dataFrameFormat.MaxVal = flowVal.As((StandardVolumeFlowUnit)dataFrameFormat.Unit).ToString();

@nkleiner nkleiner Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, was this change inspired by an issue you saw in lab?

}
}
}
}
Expand Down Expand Up @@ -461,9 +465,9 @@ public Task NewComposerMix(MfcGasComposition composerMix)
public async Task NewSetpoint(StandardVolumeFlow setpoint)
{
if(_mfcType == MfcTypeEnum.Normal)
{
{
var newSetpointCommand = new NewSetpointCommand(AssumedId, setpoint, GetFormatEntries(), FirmwareVersion);
try
try
{
var response = await Send(newSetpointCommand, TimeSpan.FromSeconds(10));
}
Expand Down Expand Up @@ -655,7 +659,9 @@ public async Task StartUpdateLoop(TimeSpan interval)
try
{
var liveData = await GetLiveData();
}
/// explicit call required here if statewatchers is not used to subscribe to the live data stream, otherwise the state will not update
UpdateLiveData(liveData);
}
catch(TimeoutException)
{
Status = new DeviceOperationalStatus { OperationalState = OperationalState.Active, Message = $"Get Live Data timed out at {DateTime.Now}" };
Expand Down Expand Up @@ -728,6 +734,9 @@ private async Task<TResult> GetResponseWithRetry<TResult, TRequest>(TRequest req
private void UpdateLiveData(LiveDataResponse liveResponse)
{
_liveData = liveResponse;
/// check on id is required if _statewatcher is used in order to avoid updating the state with a response from a different MFC than the one that is being watched

@nkleiner nkleiner Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comment above talks about this point I believe

/// if (liveResponse.Id != this.AssumedId) return;

var next = AresStateBuilder
.From(_stateSubject.Value)
.AddStruct("LiveData", b =>
Expand Down Expand Up @@ -963,7 +972,8 @@ public override async Task<CommandResult> ExecuteCommand(string command, List<De
if(!setpointFound)
return ArgumentError("NewSetpoint", "Setpoint", "number");

await NewSetpoint(StandardVolumeFlow.FromStandardCubicCentimetersPerMinute(setpoint));
_logger.LogInformation($"Attempting to set new setpoint for MFC {Name} to {setpoint} sccm");
await NewSetpoint(StandardVolumeFlow.FromStandardCubicCentimetersPerMinute(setpoint));
break;

case MassFlowControllerCommand.GetSetpoint:
Expand Down
4 changes: 3 additions & 1 deletion AlicatMFCRemastered/Parsers/MfcUnitParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ static MfcUnitParser()
// PSIA should just be absolute PSI, so I believe the unit can just be PSI
mfcUnitCache.MapUnitToAbbreviation(PressureUnit.PoundForcePerSquareInch, "PSIA");
mfcUnitCache.MapUnitToAbbreviation(VolumeFlowUnit.CubicCentimeterPerMinute, "CCM");
mfcUnitCache.MapUnitToAbbreviation(VolumeFlowUnit.LiterPerMinute, "LPM");
mfcUnitCache.MapUnitToAbbreviation(StandardVolumeFlowUnit.StandardLiterPerMinute, "SLPM");
Parser = new MfcUnitParser(new UnitParser(mfcUnitCache));
mfcUnitCache.MapUnitToAbbreviation(StandardVolumeFlowUnit.StandardCubicCentimeterPerMinute, "SCCM");
Parser = new MfcUnitParser(new UnitParser(mfcUnitCache));
}

public MfcUnitParser(UnitParser parser)
Expand Down
4 changes: 2 additions & 2 deletions AlicatMFCRemastered/UI/MfcUnitControl.razor
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
}
<div></div>

<div class="field-descriptor">Mass Flow (SCCM)</div>
<div class="field-descriptor">Mass Flow</div>
<div>@($"{ViewModel!.AlicatState.LiveData.MassFlow}")</div>
<RadzenButton Text="Tare" ButtonStyle="ButtonStyle.Secondary" Size="ButtonSize.ExtraSmall" Click="@(() => ViewModel.TareFlowCommand.Execute().Subscribe())" />

Expand All @@ -48,7 +48,7 @@
<div></div>
}

<div class="field-descriptor">Setpoint (SCCM)</div>
<div class="field-descriptor">Setpoint</div>
<div>@($"{ViewModel!.AlicatState.LiveData.Setpoint}")</div>
<div></div> <div class="field-descriptor">New Setpoint</div>
<RadzenNumeric TValue="double?"
Expand Down