Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PartDesign] Dynamic hole cut-types #3999

Merged
merged 2 commits into from
Oct 31, 2020

Conversation

berberic2
Copy link
Contributor

@berberic2 berberic2 commented Oct 25, 2020

  • Make countersink and counterbore on metric holes freely definable by user.
  • Fixed the Naming of M1.6, M2, M2.5 and M3.5
  • Added constructor for custom enums from Enums to PropertyEnumeration
  • Put definitions of cut-types (counterbore/countersink) for screwtypes into json-files for easy modification.
  • Allow users to put its own definitions in json-files in [UserDir]/Mod/PartDesign/Resources/Hole
  • Contains several examples of cut-type definition json-files that are propably not production-ready.

This uses a local copy of nlohmann::json¹ to read json-files.

Includes pullrequest #3990

Forum-Threads:


¹ This is a very nice,header-only C++ library under the MIT License (https://github.com/nlohmann/json). I copied the single-file-version and the forward-declaration-header into …/PartDesign/App/ so no new dependencies arise.

Thank you for creating a pull request to contribute to FreeCAD! To ease integration, please confirm the following:

  • Branch rebased on latest master git pull --rebase upstream master
  • Unit tests confirmed to pass by running ./bin/FreeCAD --run-test 0
  • Commit message is well-written
  • Commit message includes issue #<id> or fixes #<id> where <id> is the associated MantisBT issue id if one exists

And please remember to update the Wiki with the features added or changed once this PR is merged.
Note: If you don't have wiki access, then please mention your contribution on the 0.19 Changelog Forum Thread.


fix a problem with counterbore and countersink in PartDesign Hole feature.

It was not possible to custom define counterbores or countersinks if a
metric thread hole was selected.

Handle the cut-types None, Counterbore and Countersink euqal
regardless of type of thread and let the user customize:
None:        none
Counterbore: diameter and depth
Countersink: diameter and angle
‣ Make countersink and counterbore on metric holes freely definable by user.

‣ Fixed the Naming of M1.6, M2, M2.5 and M3.5

‣ Added constructor for custom enums from Enums to PropertyEnumeration

‣ Put definitions of cut-types (counterbore/countersink) for
  screwtypes into json-files for easy modification.

‣ Allow users to put its own definitions in json-files in
  [UserDir]/Mod/PartDesign/Resources/Hole

‣ Contains several examples of cut-type definition json-files that are
  propably not production-ready.

This uses a local copy of nlohmann::json¹ to read json-files.

__________
¹ This is a very nice,header-only C++ library under the MIT License
  (https://github.com/nlohmann/json). I copied the single-file-version
  and the forward-declaration-header into …/PartDesign/App/ so no new
  dependencies arise.

fc_target_copy_resource(PartDesignHole
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_BINARY_DIR}/share/Mod/PartDesign
Copy link
Contributor

Choose a reason for hiding this comment

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

share shouldn't be hard-coded. Use ${CMAKE_INSTALL_DATADIR} instead

FILES
${PartDesignHoleDefines}
DESTINATION
share/Mod/PartDesign/Resources/Hole
Copy link
Contributor

Choose a reason for hiding this comment

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

share shouldn't be hard-coded. Use ${CMAKE_INSTALL_DATADIR} instead.

In order to find the location at runtime use App::Application::getResourceDir().

/*!
@brief namespace for Niels Lohmann
@see https://github.com/nlohmann
@since version 1.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

In the forum you said this file is under MIT but this header file lacks of this information. So, it's best to directly add the license here, too.

According to the doxygen documentation a tag \license doesn't exist but apparently \copyright is used for this purpose.
So, \copyright MIT should be OK.

Copy link
Contributor Author

@berberic2 berberic2 Oct 31, 2020

Choose a reason for hiding this comment

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

I contacted upstream with this result:

The MIT license not really requires adding the license to all files, but I may work on this in the future.

nlohmann/json#2454

@@ -1359,3 +1300,149 @@ App::DocumentObjectExecReturn *Hole::execute(void)
return new App::DocumentObjectExecReturn(e.what());
}
}

void Hole::addCounterType(const CutDimensionSet dimensions)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be a const reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

@wwmayer wwmayer merged commit 6d1b292 into FreeCAD:master Oct 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants