speckle-sharp icon indicating copy to clipboard operation
speckle-sharp copied to clipboard

Revit: Added phase parameter

Open ks-cph opened this issue 3 years ago • 6 comments

Description

  • Fixes #1390

Type of change

  • ConertToSPeckle adds phase parameter as string

How has this been tested?

  • with some streams

Docs

  • Will be updated ASAP

ks-cph avatar Aug 01 '22 13:08 ks-cph

@connorivy sorry for the late response. Thanks for the feedback. Of course I am willing to also implement the phasing for receiving data from Speckle.

ks-cph avatar Aug 23 '22 12:08 ks-cph

@connorivy sorry for the late response. Thanks for the feedback. Of course I am willing to also implement the phasing for receiving data from Speckle.

Hey @ks-cph , thank you so much for taking the time to do this feature. I have been following this thread for quite some time, as I have a great interest in the phasing feature. Does the Receive Feature already exist? Would love to get further insight in the code, on how you manage this setting. 💯

@connorivy A question about the Merge. Will this feature be already present in Speckle 2.8? Would be very happy to receive feedback

philipp-rufi avatar Sep 11 '22 13:09 philipp-rufi

Hey @ks-cph, any updates on this? We're prepping for Hacktoberfest and updating any issues/prs that are open.

https://speckle.community/t/hacktoberfest-2022-with-speckle/3641/2

AlanRynne avatar Sep 27 '22 08:09 AlanRynne

Hey @AlanRynne It needs more change than I expected. I need to add code to every BuildingElement in the RevitConverter.

I was wondering why Speckle doesn't implement a base-class Element (or similar)?

I will work on it next week.

ks-cph avatar Sep 29 '22 07:09 ks-cph

You could leaverage the SetInstanceParameters and GetElementParams in ConversionUtils potentially...

Regarding a missing Element or RevitElement class, we have no good reasons, we attempted to keep inheritance to a minimum but could revisit that :) cc @didimitrie

teocomi avatar Sep 29 '22 11:09 teocomi

Actually, I had time today... @connorivy As we have Phase as string it may get into trouble if the value does not match with the name of a phase in the Revit project. I implemented the method ConversionUtils.GetRevitPhase() to find the Phase with the corresponding name. Is it in the correct location? And then I added a TrySetParam() in RevitConverter.SetInstanceParameters(). In this way we do not need to call the method in every BuildingElement.

Please let me know if you have any suggestions for improvements.

ks-cph avatar Sep 29 '22 11:09 ks-cph