Conversation
Also cleanup Dependencies to use more transitive package then explicit
…m.Service and Mbc.Pcs.Net.Alarm.Mediator are not complete)
So enable it with: AppContext.SetSwitch("System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization", true);
…o corresponding .NET Types in PlcCommand and AdsMapper
bqstony
changed the title
|
warm welcome as new contributor @antoninmercay |
… in Mbc.Ads.Utils
… OnConnectionStateChanged
…dency in common projects
|
Breaking change note IServiceStartable in mbc.pcs.net benutzen |
… types in the CommandInputBuilder and CommandOutputBuilder and extend the default used PrimitiveCommandArgumentHandler in PlcCommand
bqstony
changed the title
|
@stegm Kann ich das mergen? |
There was a problem hiding this comment.
PR Overview
This PR updates the Twin-ADS library to V6 and migrates the project to .NET 8.0 while performing several code cleanups and improvements. Key changes include refactoring tests for primitive read/write functionality, enhancing AdsMapper mapping methods by shifting from streams to buffers, and updating the CLI's logger integration from NLog to Microsoft.Extensions.Logging.
Reviewed Changes
| File | Description |
|---|---|
| NET/Mbc.Ads.Mapper.Test/BenchmarkMain.cs | Adds a benchmark runner for marshalling performance tests. |
| NET/Mbc.Ads.Mapper.Test/AdsBinaryAccesorFactoryTest.cs | Contains tests for reading/writing primitive types (note: a naming typo issue is present). |
| NET/Mbc.Ads.Mapper.Test/MarshallingPerformanceTest.cs | Provides benchmarks for comparing different marshalling methods. |
| NET/Mbc.Ads.Helper.ReadType/Program.cs | Implements symbol dump logic to display type metadata. |
| NET/Mbc.Ads.Mapper.Test/Reflection/FastInvokeTest.cs | Tests dynamic property and field access using reflection utilities. |
| NET/AdsMapperCli/DestinationDataObject.cs | Introduces a suppression attribute to allow multiple types in one file for test purposes. |
| NET/Mbc.Ads.Mapper.Test/AdsMapperTest.cs | Updates mapping tests to use ReadOnlySpan buffers instead of stream objects. |
| NET/AdsMapperCli/Program.cs | Refactors the CLI to use Microsoft.Extensions.Logging and updates connection parameters accordingly. |
| NET/CallAds/PcsCommand2.cs | Adjusts the Ads client type to a more abstract IAdsConnection for improved flexibility. |
| NET/Mbc.Ads.Mapper.Test/AdsSymbolReaderTest.cs | Adjusts the Ads symbol reading tests to align with updated API usage. |
| NET/CallAds/Program.cs | Updates Ads client initialization to use the new AdsClient API. |
Copilot reviewed 148 out of 148 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
NET/Mbc.Ads.Mapper.Test/AdsBinaryAccesorFactoryTest.cs:11
- Typo in class name 'AdsBinaryAccesorFactoryTest'; consider renaming it to 'AdsBinaryAccessorFactoryTest' to improve clarity.
public class AdsBinaryAccesorFactoryTest
NET/AdsMapperCli/Program.cs:91
- [nitpick] Consider rephrasing the disconnection message to 'Disconnected from TwinCat' for improved clarity.
Console.WriteLine("Disconnected to TwinCat");
…missed), remove not required EnableUnsafeBinaryFormatterSerialization, fix build warnings
…nd allign to same version
There was a problem hiding this comment.
Copilot reviewed 145 out of 154 changed files in this pull request and generated 1 comment.
Files not reviewed (9)
- NET/AdsMapperCli/AdsMapperCli.csproj: Language not supported
- NET/Build/Mbc.CodeAnalysis.ruleset: Language not supported
- NET/Build/Mbc.Pcs.Net.TwinCat.EventLog.nuspec: Language not supported
- NET/Build/stylecop.json: Language not supported
- NET/CallAds/CallAds.csproj: Language not supported
- NET/Directory.Build.props: Language not supported
- NET/Mbc.Ads.Helper.ReadType/Mbc.Ads.Helper.ReadType.csproj: Language not supported
- NET/Mbc.Ads.Helper.ReadType/Properties/launchSettings.json: Language not supported
- NET/Mbc.Ads.Mapper.Test/Mbc.Ads.Mapper.Test.csproj: Language not supported
| }; | ||
|
|
||
| while (keepRunning) { } | ||
| while (keepRunning) |
There was a problem hiding this comment.
The infinite loop used for termination is busy waiting without any delay, which may lead to high CPU usage. Consider adding a small delay (e.g., Thread.Sleep(10)) inside the loop.
There was a problem hiding this comment.
Das finde ich super wenn es das einzige Makel ist das du gefunden hast :)
3 participants
see #7
the not merged #5 is also included
when this PR is merged, the PR #6 will be obsolete!
thx all for help