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

Photon Feeder - No PartId Prevents Configuration Save #1557

Open
warasilapm opened this issue May 11, 2023 · 7 comments
Open

Photon Feeder - No PartId Prevents Configuration Save #1557

warasilapm opened this issue May 11, 2023 · 7 comments

Comments

@warasilapm
Copy link

warasilapm commented May 11, 2023

Problem Report

Describe the Issue

When using the photon feeder type with the opulo 8mm feeder, if the feeder does not have a PartId attached to it, the machine configuration will not save. See error below.

Steps to Reproduce

  1. Start OpenPnP, and power machine on.
  2. Add manually or by searching in the photon feeder global config tab a photon feeder.
  3. Do not select a PartId for the feeder.
  4. Attempt to save configuration using the File menu.
  5. The machine configuration does not save and generates the error shown below.

Expected Result

The machine configuration to save.

Actual Result

The machine configuration does not save successfully. Changes to other areas of the machine configuration are also not affected by this change; it appears any and all changes do not save properly.

Notes, Log Files, Screen Captures, Videos, etc. to Show the Issue

image

I also attempted to create this issue by creating a new ReferenceStripFeeder. However, it would appear that FIDUCIAL-HOME is always selected by default. Maybe this is the expected safe state for feeder part IDs?

@Jnesselr
Copy link
Collaborator

This error is in the AbstractFeeder and it's part of the base feeder. I suspect this is because OpenPnP generally considers a feeder as "parts you care about right now" and doesn't usually deal with feeders that aren't there to serve a part (whereas our feeders stay in OpenPnP even if they're off the machine) or may not have an associated part.

@vonnieda @markmaker Any reason why required is true here for the part id?

@markmaker
Copy link
Collaborator

Hi @Jnesselr,

This was defined before my time, but I think a feeder just always needs a part assigned. When you create a new feeder, and no part is given, it defaults to the very first part, here:

feeder.setPart(part == null ? Configuration.get().getParts().get(0) : part);

Plus the UI does not allow to select a null part.

So when you create a PhotonFeeder automatically, you need to make sure the same happens, for instance here:

otherFeeder = new PhotonFeeder();
otherFeeder.setHardwareId(response.uuid);
Configuration.get().getMachine().addFeeder(otherFeeder);

_Mark

@bdebyl
Copy link

bdebyl commented May 16, 2023

The way I've gotten around this, on my own feeders unrelated to the Photon feeder, is I just have a specific "dummy" part that is not used by any job and marked as such -- can be anything valid really, Fiducials may even work.

@warasilapm
Copy link
Author

FIDUCIAL-HOME is what my setup defaults to on all other feeders.

@Jnesselr
Copy link
Collaborator

I feel like forcing there to be a part doesn't make sense for this type of feeder. If I make it optional and make sure there's no crashes, is that acceptable? I feel like anything else is a workaround.

@markmaker
Copy link
Collaborator

If I make it optional and make sure there's no crashes, is that acceptable?

That would be very welcome! 💯 😎

It always irked me that this random first part is assigned.

But do not underestimate the needed work. You'd need to ...

  1. Check and sanitize each and every call to Feeder.getPart() and Feeder.partId.
  2. Add the null Part selection to the GUI (ComboBox, may be handled as empty String), so it is properly displayed, and can also be set.
  3. Check where other feeders are created programmatically, and make them default to the null part too.

_Mark

@Jnesselr
Copy link
Collaborator

Yep, that about sums up what I expected the work to be as well. Glad we're on the same page. I'll try to schedule this task in. I know there's at least one other PR I'll need to send y'all's way too.

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

No branches or pull requests

4 participants