DynamoDS / DynamoRevit

Dynamo Libraries for Revit
https://dynamobim.org
336 stars 187 forks source link

ElementWrapper Missing with Get Tap Of Pipe #2903

Closed chuongmep closed 1 year ago

chuongmep commented 1 year ago

If this issue is not a bug report or improvement request, please check the Dynamo forum, and start a thread there to discuss your issue.

Dynamo version

2.12

Revit version

2022

Operating system

Windows 10

What did you do?

Create a ZeroTouch Return Tap Of Pipe,

 public static Elements.Element GetTap(Revit.Elements.Element pipe)
    {
        Autodesk.Revit.DB.Plumbing.Pipe? pipeInternalElement = pipe.InternalElement as Autodesk.Revit.DB.Plumbing.Pipe;
        FamilySymbol familySymbol = pipeInternalElement!.PipeType.Tap;
        Revit.Elements.Element tap = familySymbol.ToDynamoType();
        return tap;
    }

Then :

Warning: Pipe.GetTap operation failed. The call is ambiguous between the following methods or properties: 'Revit.Elements.ElementWrapper.Wrap(Autodesk.Revit.DB.RoofType, bool)' and 'Revit.Elements.ElementWrapper.Wrap(Autodesk.Revit.DB.ScheduleSheetInstance, bool)' ảnh

What did you expect to see?

It will be work perfect

What did you see instead?

(Fill in here)

chuongmep commented 1 year ago

Hi @Amoursol , any new update for this , same when we create new union fitting .

TransactionManager.Instance.ForceCloseTransaction();
        using Autodesk.Revit.DB.Transaction tran = new Autodesk.Revit.DB.Transaction(firstPipe.InternalElement.Document, "Create Union Fitting");
        tran.Start();
        Connector? c1 = firstPipe.InternalElement.GetConnectorCloset(secondPipe.InternalElement);
        Connector? c2 = secondPipe.InternalElement.GetConnectorCloset(firstPipe.InternalElement);
        ElementId unionFitting = firstPipe.InternalElement.Document.Create.NewUnionFitting(c2, c1).Id;
        int idValue = unionFitting.IntegerValue;
        tran.Commit();

in this case unionFitting allway return null.

QilongTang commented 1 year ago

@wangyangshi Are you aware of this, should we forward this issue to Revit API team or?

wangyangshi commented 1 year ago

Sorry for the late reply, I will look into this today, check if it is a D4R issue or Revit API related.

chuongmep commented 1 year ago

I also posted a topic in forum to explain for this: https://forum.dynamobim.com/t/newunionfitting-allway-return-null-in-dynamo-revit/85939/1

wangyangshi commented 1 year ago

Hi @chuongmep what is the function 'ToDynamoType'? I am trying to reproduce your issue, cannot find it in d4r codes?

chuongmep commented 1 year ago

Hi @wangyangshi it is ToDSType(True)

wangyangshi commented 1 year ago

@chuongmep image I reproduced this issue, what I found is: 1 the property Tap is null, 2 then in the function ToDSType, as it calles ElementWrapper.Wrap, for the null Element, it doesn't know which is the right wrap function should use, so throw the warn. Do you think the null is abnormal in 1 ? Or we should make the warn message clear, for example adding a check in the function ToDSType before calling wrap.

wangyangshi commented 1 year ago

pipeInternalElement.PipeType.Tap is FamilySymbol in fact this wrapper exists. image

chuongmep commented 1 year ago

@wangyangshi that is correct issue I want report, I have many methods is same problem now.

wangyangshi commented 1 year ago

1 Tap is null is abnormal, we need to investigate why it is null? 2 tap is null is expected, we just need to improve the warn message? I am not very clear which one do you want, or both two?

wangyangshi commented 1 year ago

if you care about is 1, I will contact with related Revit API owner to seek for reasons, I am not very clear about MEP related. if you care about is 2, we can do some improvement in d4r codes.

chuongmep commented 1 year ago

Hi @wangyangshi, I need confirm my issue for clear :

wangyangshi commented 1 year ago

OK, thank you, I will post these to related API teams.

wangyangshi commented 1 year ago

Hi @chuongmep what is the function 'GetConnectorCloset', I cannot find it in Revit API, we need to reproduce the NewUnionFitting issue, thank you again.

chuongmep commented 1 year ago

@wangyangshi I want provide for you some file to quick check : Rvt2022 : https://1drv.ms/u/s!AmKukXZ0HxiElqxoqongELn_BG4fEA?e=KckFGi Scripts : https://1drv.ms/u/s!AmKukXZ0HxiElqxpOrYfLWiHXxH-YA?e=dA7BEo

wangyangshi commented 1 year ago

I see 2 links are both RVT files? image

chuongmep commented 1 year ago

I see 2 links are both RVT files? image

I updated, can help me try again ?

wangyangshi commented 1 year ago

Yes, it is the DYN file now. image

wangyangshi commented 1 year ago

Hi @chuongmep I contacted with other MEP related teams, got the reply, template file has no Pipe Tap fitting by default. And user need to load the tap to the project, and then set it to pipe type, thus user can get the tap element

  1. In the project, load the tap family;
  2. Select or Create Pipe -> Pipe Type -> Routing Preference -> change "Junction" to the tap family, and then change the junction type to "Tap"

I haven't tried this yet, just wander do you miss these steps then return the null tap?

chuongmep commented 1 year ago

@wangyangshi, many thanks for @wangyangshi about tap, I resolved from my side, how about issue with create union fitting ?

wangyangshi commented 1 year ago

@chuongmep the API NewUnionFitting calls RbsEditorUtils::insertUnion in Revit, in fact the union element is created, but in this function, there is a check, if union element type is no REPT_Union, it will return null. I use your RVT&DYN file to test, the union element type is REPT_Transition, so although union element created successfully, it still returns null.

chuongmep commented 1 year ago

@wangyangshi, it means is this is correct null with Revit API ?

wangyangshi commented 1 year ago

Yes, I see those codes hasn't been changed for many years, I think this is by design. in the forum someone recommends using NewTransitionFitting, because your return element Type is REPT_Transition, from the function name I think this should be the suitable method for you, would you please have a try?

wangyangshi commented 1 year ago

image all these NewXXXFitting methods check element type if match then return the element, if not match then return null, so you may need to use the corresponding function.

chuongmep commented 1 year ago

Thanks, @wangyangshi, this is clear with me now, thanks for your supported, maybe I need some trick to help user get it.