Skip to content
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 74 additions & 1 deletion Robot_Adapter/CRUD/Update/Loads/LoadCombinations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* along with this code. If not, see <https://www.gnu.org/licenses/lgpl-3.0.html>.
*/

using System;
using System.Collections.Generic;
using BH.oM.Base;
using BH.oM.Structure.Elements;
Expand All @@ -29,6 +30,7 @@
using BH.oM.Structure.Loads;
using BH.oM.Physical.Materials;
using BH.oM.Adapter;
using BH.Engine.Base;
using RobotOM;

namespace BH.Adapter.Robot
Expand All @@ -42,7 +44,78 @@ public partial class RobotAdapter
protected bool Update(IEnumerable<LoadCombination> loadCombinations)
{
bool success = true;
success = ICreate(loadCombinations);

m_RobotApplication.Project.Structure.Cases.BeginMultiOperation();

foreach (LoadCombination lComb in loadCombinations)
{
//Check combination itself is not null
if (!CheckNotNull(lComb))
continue;
Comment on lines +52 to +54
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's null don't we want to return a error stating as such - and then returning false?


// Use the Number property directly and try to get the combination
int combinationId = lComb.Number;

// Check if the RobotId matches the LoadCombination Number
int robotId = GetAdapterId<int>(lComb);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int robotId = GetAdapterId<int>(lComb);
int robotId = Query.HasAdapterIdFragment(lComb, typeof(RobotId)) ? GetAdapterId<int>(lComb) : 0;

You should check if lComb has a Fragment first, otherwise you will get two warnings/notes for the same thing:

Image


// If the LoadCombination doesn't have a RobotId, assign it from the Number
if (robotId == 0)
{
this.SetAdapterId(lComb, combinationId);
robotId = combinationId;
Engine.Base.Compute.RecordNote($"LoadCombination with number {combinationId} did not have a RobotId. RobotId has been set to the LoadCombination number.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Engine.Base.Compute.RecordNote($"LoadCombination with number {combinationId} did not have a RobotId. RobotId has been set to the LoadCombination number.");
Engine.Base.Compute.RecordWarning($"LoadCombination with number {combinationId} did not have a RobotId. RobotId has been set to the LoadCombination number.");

Warning given you have made changes to the users object.

}
else if (robotId != combinationId)
{
Engine.Base.Compute.RecordError($"Load combination has mismatched IDs: RobotId = {robotId}, Number = {combinationId}. Using Number property for update.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Engine.Base.Compute.RecordError($"Load combination has mismatched IDs: RobotId = {robotId}, Number = {combinationId}. Using Number property for update.");
Engine.Base.Compute.RecordWarning($"Load combination has mismatched IDs: RobotId = {robotId}, Number = {combinationId}. Using Number property for update.");

I would make this a warning, to make the user aware of a change of approach. Errors are usually when something is not going to work.

}

// Get the existing combination from Robot (following pattern from Loadcases Update method)
RobotCaseCombination rCaseCombination = m_RobotApplication.Project.Structure.Cases.Get(combinationId) as RobotCaseCombination;
if (rCaseCombination == null)
{
Engine.Base.Compute.RecordWarning("Load combination with number " + combinationId.ToString() + " does not exist in Robot. Load combination could not be updated!");
success = false;
continue;
Comment on lines +78 to +80
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Engine.Base.Compute.RecordWarning("Load combination with number " + combinationId.ToString() + " does not exist in Robot. Load combination could not be updated!");
success = false;
continue;
Engine.Base.Compute.RecordError("Load combination with number " + combinationId.ToString() + " does not exist in Robot. Load combination could not be updated!");
return false;

Suggest we return an error here and exit the method given you cannot do anything with a null rCaseCombination.

}

// Update the combination name if provided
if (!string.IsNullOrWhiteSpace(lComb.Name))
rCaseCombination.Name = lComb.Name;

// Clear existing case factors by deleting them individually
// Note: Robot API requires deleting case factors in reverse order to avoid index shifting
for (int i = rCaseCombination.CaseFactors.Count; i >= 1; i--)
{
rCaseCombination.CaseFactors.Delete(i);
}
Comment on lines +89 to +92
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not put this inside the if statement below? That way you don't clear all the factors of a LoadCombination that has no Loadcases.


// Add new case factors from the BHoM LoadCombination
if (lComb.LoadCases != null && lComb.LoadCases.Count > 0)
{
for (int i = 0; i < lComb.LoadCases.Count; i++)
{
//Check tuple as well as case not null
if (CheckNotNull(lComb.LoadCases[i], oM.Base.Debugging.EventType.Error, typeof(LoadCombination)) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add a using BH.oM.Base.Debugging; statement you can simplify this.

I would also add using
using BH.oM.Adapters.Robot; using BH.Engine.Adapter;

CheckNotNull(lComb.LoadCases[i].Item2, oM.Base.Debugging.EventType.Error, typeof(LoadCombination)))
{
System.Tuple<double, ICase> loadcase = lComb.LoadCases[i];
rCaseCombination.CaseFactors.New(lComb.LoadCases[i].Item2.Number, lComb.LoadCases[i].Item1);
}
}
}
else
{
Engine.Base.Compute.RecordWarning("Load combination with number " + combinationId.ToString() + " has no load cases. The combination has been cleared of all case factors.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Engine.Base.Compute.RecordWarning("Load combination with number " + combinationId.ToString() + " has no load cases. The combination has been cleared of all case factors.");
Engine.Base.Compute.RecordWarning("Load combination with number " + combinationId.ToString() + " has no load cases.");

Then we don't delete the factors.

}

// Set the adapter ID to maintain the connection between BHoM and Robot objects
this.SetAdapterId(lComb, lComb.Number);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line needed? You have already updated the object id when it didn't match the load combination number?

}

m_RobotApplication.Project.Structure.Cases.EndMultiOperation();

return success;
}

Expand Down