Skip to content
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.

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

AdsOverMQTT hijacks previous exception types when failed to connect #248

Open
legrab opened this issue Mar 5, 2025 · 2 comments
Open

AdsOverMQTT hijacks previous exception types when failed to connect #248

legrab opened this issue Mar 5, 2025 · 2 comments

Comments

@legrab
Copy link

legrab commented Mar 5, 2025

The recently added AdsOverMQTT connection type now throws a barebone Exception with some message as AmsServer 'AdsOverMqtt' is not set!

While I understand that there is a way to configure the connection to be ADS-only, I still believe that the exception here should be some sort of AmsServerException or other identifiable implementation and not a random Exception to handle it distinctly.

Note that previously, with no connection setup, it was either LoopbackNotRegisteredException or AdsServerException that was thrown here and it was possible to rely on that to handle the cases. Now one would need to rely on the message of the exception which is undesired.

Please consider using some existing exception type such as AdsServerException or add a new type instead (as suggested above: AmsServerException sounds legit to me) instead of falling back to generic Exception type.

System.Exception: AmsServer 'AdsOverMqtt' is not set!
   at TwinCAT.Ads.AmsServerComposition.GetServerFactory(ChannelProtocol channelProtocol)
   at TwinCAT.Ads.Server.AmsServer.createDIChannel(ChannelProtocol requestedProtocol, ChannelPortType requestedPortType)
   at TwinCAT.Ads.Server.AmsServer.createServerImplementation(ChannelProtocol requestedProtocol, ChannelPortType requestedPortType)
   at TwinCAT.Ads.Server.AmsServer.ConnectServer()
   at TwinCAT.Ads.Server.AdsServer.ConnectServer()
   at TwinCAT.Ads.AdsClient.Connect(AmsAddress address)
   at TwinCAT.Ads.AdsConnection.OnConnect()
   at TwinCAT.Ads.AdsConnection.Connect()
   at TwinCAT.Ads.AdsSessionBase.OnConnect(Boolean reconnect)
   at TwinCAT.Session.Connect()
@pbruenn
Copy link
Member

pbruenn commented Mar 6, 2025

Hmm, when I browse through the repository I can't find “AmsServer” anywhere. I somehow have the strong suspicion that you have landed in the wrong repository here. How did you end up here? I can only assume you are actually using .NET?

@RalfHeitmann
Copy link

Yes, that is .NET implementation. About this issue there is already a discussion here:

Beckhoff/TF6000_ADS_DOTNET_V5_Samples#65

The Exception is not intended to leak, but nevertheless it will be changed to type AmsServerException and improved error message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants