Skip to content

Conversation

@marcin-nrobotics
Copy link

Issue:
The pure sketch functionality did not work when the config file was not in the same folder as the STL files, as is default in the current version of onshape_to_robot (by default STL are stored in a subfolder called /assets)

Solution:
This PR has a three step procedure for finding the config file.

  1. If a user provides the config file as an argument (--config), it attempts to use this location
  2. Otherwise, it looks in the STL folder (which I imagine was the previous default location)
  3. Failing that, it looks one dir above

To facilitate another command-line argument, parsing was moved to argparse. This moved everything up one indent, hence the slightly messy commit, apologies.

In conjunction with PR#175 this fixes PureShape functionality in 1.7.7.

@Gregwar
Copy link
Contributor

Gregwar commented Nov 10, 2025

Hello and thanks for this proposition

I have another suggestion about that, maybe the path to config.json should be included in the .part file that is generated with metadata next to STLs by the import. It might be easier to use.

What do you think?

@marcin-nrobotics
Copy link
Author

I agree that giving the location of the config.json file as a command line arg is a bit of a hack and I would also rather avoid that.

However, I would be worried about including it in a .part file as well. If the location changes, all the .part files would also have to change, and there is currently no mechanism for that, which is likely to lead to breaking changes down the line.

IMO it would be best to enforce a single default location for the config file, e.g. in the project base folder, and build everything else around that. I think all the current functionality would work with this approach?

For this PR, this would mean removing the command line arg and keeping the dir checking code.

WDYT @Gregwar ?

@Gregwar
Copy link
Contributor

Gregwar commented Nov 12, 2025

Currently, assets_directory allow for assets to be located elsewhere than a subdirectory from where config.json is. However, I think it is acceptable to assume that it will, in general, actually be the case.

Remove the --config flag and I'll merge the PR

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