Duct Elements Convert Method Modified to Consider Disciplines #1260
Conversation
travispotterBH
added
type:feature
travispotterBH
requested review from
pawelbaran,
kayleighhoude,
alexissantella,
michal-pekacki and
vietle-bh
travispotterBH
changed the title
There was a problem hiding this comment.
I am afraid you are jumping too far ahead with this PR @travispotterBH. The current system around the Revit converts is based on the concept of having one convert per object type. This means you cannot have 2 types of converts to MEP ducts depending on discipline input (see the code comments).
Instead, you would need to start from adding MEP objects to the Physical namespace (Physical_oM) to then start supporting them in Revit_Toolkit. I hope this makes sense - even if taking a shortcut sounds tempting in short term, I am pretty sure we would need to pay double price once we realise the amount of extra effort required to make it work plus potentially refactor in long term.
| @@ -44,7 +45,7 @@ public static partial class Convert | |||
| [Input("settings", "Revit adapter settings.")] | |||
| [Input("refObjects", "A collection of objects processed in the current adapter action, stored to avoid processing the same object more than once.")] | |||
| [Output("ducts", "List of BHoM duct objects converted from a Revit duct elements.")] | |||
| public static List<BH.oM.MEP.System.Duct> DuctFromRevit(this Autodesk.Revit.DB.Mechanical.Duct revitDuct, RevitSettings settings = null, Dictionary<string, List<IBHoMObject>> refObjects = null) | |||
| public static List<BH.oM.MEP.System.Duct> DuctFromRevit(this Autodesk.Revit.DB.Mechanical.Duct revitDuct, RevitSettings settings = null, Dictionary<string, List<IBHoMObject>> refObjects = null, Discipline discipline = Discipline.Physical) | |||
There was a problem hiding this comment.
Discipline method parameter should not be added to this method - if you need it, please make this method private and then expose it via one-line public converts for physical and MEP disciplines that would call it.
| @@ -160,7 +161,7 @@ public static BH.oM.Geometry.Line LocationCurveColumn(this FamilyInstance family | |||
| [Input("mepCurve", "Revit MEPCurve to be queried.")] | |||
| [Input("settings", "Optional, the RevitSettings to be used.")] | |||
| [Output("locationCurveMEP", "BHoM lines list queried from the MEPCurve.")] | |||
| public static List<BH.oM.Geometry.Line> LocationCurveMEP(this MEPCurve mepCurve, RevitSettings settings = null) | |||
| public static List<BH.oM.Geometry.Line> LocationCurveMEP(this MEPCurve mepCurve, RevitSettings settings = null, Discipline discipline = Discipline.Physical) | |||
There was a problem hiding this comment.
This is fine, there is no limitations on the Query methods.
|
Will not planned to be resolved in this sprint. |
I did not know the best place for this PR, or what was the best name of the branch, so I am prepared to remake this after comments.
Issues addressed by this PR
Closes #1214
Closes BHoM/BHoM#1403
Attempts to address issue of duct objects being converted from Revit only as their analytical representation, which splits the ducts on all connected connectors (mostly only an issue on ducts with tap style fittings)
Try to Push to JSON a selected duct run with runs connected by taps and elbows like below:
export the same duct run as a physical discipline and as an environmental discipline.
On the case of runs with tap fittings:
Physical Discipline should export just as the single, not separated by midpoint, Revit Element.
Environmental Discipline should export as the separated by midpoint element and should have multiple bhomobjects of type Duct of the same Revit Element Id.
Runs without tap fittings should export similar to the Physical discipline would.
This was achieved by changing the
FromRevitConvertMethod for ducts calledDuctFromRevitto take in the discipline provided, and that is passed into theQuery.LocationCurveMEPmethod which then will either split or not split the duct line based on the discipline given.This method of doing this required edits to the
FromRevitConvert method that handlesElementtypes where it was required the new Invocation of theconvertMethodwould be able to handle additional parameters, namely theDisciplinetype. This further required a change toQuery.AllConvertMethodsto check for methods that have between three and four parameters instead of what was previous set to three.@pawelbaran I recognize that this may not be the most optimal solution, and I welcome your comments/suggestions/assistance.
@kayleighhoude can you give some indication on how well this suits your needs.
Test files
Revit file: Any revit file with some ducts and fittings connected in a duct run
Expected Results:
https://burohappold.sharepoint.com/:f:/r/sites/BHoM/02_Current/12_Scripts/02_Pull%20Request/BHoM/Revit_Toolkit/Revit_Core_Engine-%231260-DuctElementsCreateMethodModifiedtoConsiderDisciplines?csf=1&web=1&e=3Gpp5P
Changelog
Additional comments