CNX-9228 : minimal working revit receive operation that receives levels #3286

connorivy · 2024-04-16T17:58:38Z

Description & motivation

Changes:

To-do before merge:

Invoking common receive operation located in Speckle.Connectors.Utils.Operations with custom RevitHostObjectBuilder
- Wrapped entire thing in RevitTask to execute in API context
- Brought over the transactionManager (with slightly different method names) to provided transaction manipulation
- Brought over the ErrorPreprocessingService (formerly called the ErrorEater) which can be used to either deal with Revit errors or alert the user
- Brought over the "YieldToUIThread" method to pump the event loop and keep the UI refreshed. The previous one was based on the Avalonia Dispactcher, so this new one would need some testing to see if it is really working.
RevitConverterToHost which is similar to other host app receive converters
Brought over level conversion to Revit except left out a chunk of code dealing with receiving levels that have a "referenceOnly" property that I'm not fully understanding. Matteo is probably the best person to explain this.

Screenshots:

Validation of changes:

Checklist:

My pull request follows the guidelines in the Contributing guide?
My pull request does not duplicate any other open Pull Requests for the same update/change?
My commits are related to the pull request and do not amend unrelated code or documentation.
My code follows a similar style to existing code.
I have added appropriate tests.
I have updated or added relevant documentation.

References

…evel

BovineOx

We should probably be a bit picky about which bits we pull across from the old converters/connectors as we're getting into areas we want to make good/better and we will need more alignment

DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/ReceiveBinding.cs

DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/AutofacUIModule.cs

...Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs

BovineOx · 2024-04-17T14:58:37Z

DUI3-DX/Converters/Revit/Speckle.Converters.RevitShared/RevitConverterToHost.cs

+    return result;
+  }
+
+  private ISpeckleObjectToHostConversion? GetToHostConversion(Type? targetType)


Why would Type be null?

BovineOx · 2024-04-17T15:07:12Z

...erters/Revit/Speckle.Converters.Revit2023.DependencyInjection/AutofacRevitConverterModule.cs


    // factory for conversions
    builder
      .RegisterType<Factory<string, IHostObjectToSpeckleConversion>>()
      .As<IFactory<string, IHostObjectToSpeckleConversion>>();
+    builder
+      .RegisterType<Factory<string, ISpeckleObjectToHostConversion>>()
+      .As<IFactory<string, ISpeckleObjectToHostConversion>>();

    // POC: do we need ToSpeckleScalingService as is, do we need to interface it out?
    builder.RegisterType<ToSpeckleScalingService>().AsSelf().InstancePerLifetimeScope();


These should all be interfaced out

BovineOx · 2024-04-17T15:08:37Z

DUI3-DX/Converters/Revit/Speckle.Converters.RevitShared/Services/ErrorPreprocessingService.cs

+
+namespace Speckle.Converters.RevitShared.Services;
+
+public class ErrorPreprocessingService : IFailuresPreprocessor


This seems completely out of scope tbh, we need to design and POC the reporting and this far we've largely been ignoring it. I think this should come out for now - was this a copy and paste?

this was copy paste. Removing for now, but this is a decently important class in Revit that we should make sure we reimplement. I've added a POC comment.

BovineOx · 2024-04-17T15:09:51Z

DUI3-DX/Converters/Revit/Speckle.Converters.RevitShared/Services/ToHostScalingService.cs

+    return UnitUtils.ConvertToInternalUnits(value, UnitsToNative(units));
+  }
+
+  private static ForgeTypeId UnitsToNative(string units)


Isn't this being done elsewhere?

I thought the same thing but I don't think so. In the contextStack, we have an IHostToSpeckleUnitConverter which goes from hostUnit to string, but not string to HostUnit. Maybe we should add this to that interface instead?

BovineOx · 2024-04-17T15:10:37Z

...-DX/Converters/Revit/Speckle.Converters.RevitShared/Services/TransactionManagementService.cs

+/// </summary>
+public sealed class TransactionManagementService : ITransactionManagementService
+{
+  private readonly Lazy<RevitConversionContextStack> _lazyContextStack;


Any reason for making this lazy? Is this essentially a singleton with the context of a conversion?

this was needed because it was being instantiated in a dependency of the binding (right when Revit was opened before there was an active document) which was causing an error because the context stack is instantiated using the active document. However, this is no longer needed after moving the uow out to the binding.

BovineOx · 2024-04-17T15:13:10Z

...-DX/Converters/Revit/Speckle.Converters.RevitShared/Services/TransactionManagementService.cs

+    return TransactionStatus.Uninitialized;
+  }
+
+  private void HandleFailedCommit(TransactionStatus status)


This feels like it maybe doing too much and is using SpeckleLog directly.

you are right. Removed the error handling and added POC comments

…evel

AlanRynne · 2024-04-22T19:53:35Z

Moving this to draft as we'll get receive in later on

Connor Ivy added 3 commits April 11, 2024 16:24

basic receive functionality

752ab4e

minimal working receive operation

c9e73ce

Merge remote-tracking branch 'origin/dui3/alpha' into CNX-9228-RevitL…

f852b82

…evel

connorivy changed the title ~~Cnx 9228 : minimal working revit receive operation~~ Cnx 9228 : minimal working revit receive operation that receives levels Apr 16, 2024

add POC comment

4117f42

connorivy assigned AlanRynne and BovineOx Apr 17, 2024

BovineOx requested changes Apr 17, 2024

View reviewed changes

Connor Ivy added 2 commits April 19, 2024 13:44

Merge remote-tracking branch 'origin/dui3/alpha' into CNX-9228-RevitL…

78a95b2

…evel

address ian's comments

23575a1

AlanRynne marked this pull request as draft April 22, 2024 19:53

AlanRynne changed the title ~~Cnx 9228 : minimal working revit receive operation that receives levels~~ CNX-9228 : minimal working revit receive operation that receives levels Apr 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CNX-9228 : minimal working revit receive operation that receives levels #3286

CNX-9228 : minimal working revit receive operation that receives levels #3286

connorivy commented Apr 16, 2024

BovineOx left a comment

BovineOx Apr 17, 2024

BovineOx Apr 17, 2024

BovineOx Apr 17, 2024

connorivy Apr 19, 2024

BovineOx Apr 17, 2024

connorivy Apr 19, 2024

BovineOx Apr 17, 2024

connorivy Apr 19, 2024

BovineOx Apr 17, 2024

connorivy Apr 19, 2024

AlanRynne commented Apr 22, 2024


		namespace Speckle.Converters.RevitShared.Services;

		public class ErrorPreprocessingService : IFailuresPreprocessor

CNX-9228 : minimal working revit receive operation that receives levels #3286

Are you sure you want to change the base?

CNX-9228 : minimal working revit receive operation that receives levels #3286

Conversation

connorivy commented Apr 16, 2024

Description & motivation

Changes:

To-do before merge:

Screenshots:

Validation of changes:

Checklist:

References

BovineOx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlanRynne commented Apr 22, 2024