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

Feature/Actuator based Pick+Place, Contact Probing Nozzle and independant Valve Switching #859

Merged

Conversation

markmaker
Copy link
Collaborator

@markmaker markmaker commented Jun 23, 2019

Description

EDIT: this PR has changed guite a bit. See the discussion.

  • removed pick() and place() methods from the drivers.
  • removed GCodeDriver PICK_COMMAND/PLACE_COMMAND fragments.
  • vacuum valves actuated by Actuators, controlled by Nozzle pick() and place()
  • vacuum pump added to Head, actuated by Actuator
  • valve actuation in turn actuates the pump if needed
  • removed GCodeDriver PUMP_ON_COMMAND, PUMP_OFF_COMMAND.
  • adapted testing to new actuator based valve (as much as I understand)
  • changed all Wizard Actuator references into ComboBoxes for easy selection
  • added new ContactProbeNozzle subclass to OpenPNP allowing pick and place with contact sensing Actuator.

Justification

As discussed previously in PR #855, a solution is needed for machines that use Z contact probing nozzles for pick an place. When the nozzle is lowered to the part on a pick / to the PCB on a placement, the nozzle will stop automatically when a certain contact force is sensed (Liteplacer style).

The OpenPNP 1.0 solution involved adding probing G-code to the PICK_COMMAND and the PLACE_COMMAND. But this was no longer possible for OpenPNP 2.0 because the pick() and place() methods are also used for valve switching during the isPartOff() vacuum test. Obviously, no probing should happen then.

To cleanly separate the valve switching from the pick() and place() semantics, valve switching is moved to the vaccum actuator. Both reading the vacuum level and switching the vacuum valve is now implemented there. Users can then use the actuator to test vacuum valve switching and to determine vacuum levels for the isPartOn() and isPartOff() ranges they must enter in the configuration.

At the same time Nozzle pick() and place() are really what the name implies and can be overridden to do more than just valve switching in a subclass. We're back where this PR originated: the ContactProbingNozzle sub-class.

Some machines switch the vacuum pump on and off as needed (as long as at least one part is on a nozzle). In this PR the pump is therefore also modelled as a separate actuator with boolean control. The goal is to move towards a design, where more machine elements are modelled as separate objects rather than hide them inside GCode and entangle them with other functionality. A more general Actuator based redesign is achieved, where the user can test and use each machine element separately (for setup in the Machine Controls and for custom scripting). The user must only once learn how to setup an actuator and can reuse that knowledge many times. See the discussion for more detail on how this solution evolved.

The PR contains both the redesign and the new ContactProbingNozzle as its justification. The second is clearly separated in its own new file (plus one line added for registration).

Instructions for Use

Vacuum actuator

On your GCode driver choose the existing vacuum Actuator, define the ACTUATE_BOOLEAN_COMMAND. An example (for Smoothieware) could be:

{True:M10 ; open solenoid valve}
{False:M11 ; close solenoid valve}

This addition is also useful for the user to experiment with the valve and vacuum levels, using the Actuators tab and buttons in the Machine Controls.

grafik

Pump actuator

Create a new actuator on your head.
grafik
grafik
grafik

Set it as the pump actuator:
grafik

On your GCode driver choose the new pump Actuator and define the ACTUATE_BOOLEAN_COMMAND.

grafik
An example (for Smoothieware) could be:

{True:M106 ; turn on pump}
{False:M107 ; turn off pump}

ContactProbeNozzle

If you have a machine with a nozzle tat can sense when it makes contact tith the nozzle tip (the Liteplacer has it), you can use the new ContactProbeNozzle.

Instead of using the ReferenceNozzle, create a ContactProbeNozzle.
grafik

If you already have one or more ReferenceNozzles set up, close OpenPNP, edit your machine.xml and replace all

<nozzle class="org.openpnp.machine.reference.ReferenceNozzle"
with
<nozzle class="org.openpnp.machine.reference.ContactProbeNozzle"

Start OpenPNP again and add a new probing Actuator for each Nozzle. On the Nozzle, assign the probe:
grafik

On your GCode driver choose the new contact probing Actuator and define the ACTUATE_BOOLEAN_COMMAND for probing.

grafik

An example (for Smoothieware) could be (Note, we are using M400 followed by M114.2 instead of plain M114 because Smoothieware does not report the rotation axis with M114):

{True:G38.2 Z-4 F1200   ; Probe down max. 4mm for contact with picked/placed part }
{True:M400          ; Wait until machine has stopped }
{True:M114.2        ; Report current realtime position, as M114 does not report rotation }
{False:G91 G0 Z1 G90; Retract 1mm }
{False:M400         ; Wait until machine has stopped }
{False:M114.2       ; Report current realtime position, as M114 does not report rotation }

You need to define the POSITION_REPORT_REGEX to read back the probed Z i.e. to get OpenPNP back in sync with the machine position after probing.

grafik

For Smoothieware this could be:

^ok MCS: X:(?<x>-?\d+\.\d+) Y:(?<y>-?\d+\.\d+) Z:(?<z>-?\d+\.\d+) A:(?<rotation>-?\d+\.\d+).*

To test the contact probing, move your nozzle tip on top of an object not quite touching it. Then go to the machine controls, switch to the Actuators tab and click on the Probe Actuator:
grafik

You can now use the On and Off buttons to probe and retract. If you are too far above the contact surface, the machine will halt because it did not reach the contact within the 4mm given in the Gcode example above (adapt this value to your machine i.e. the contact probe travel/spring). This is another safety check halting the machine if something is not right (such as the feeder Z position not set right).

You should set your PCB Z and the feeder pick location Z so that the nozzle tip is not quite touching the part. The probing will then allow for variations within the probing distance, so you don't need super precision in your feeders etc. and your setup.

Note the probed part Z height is not yet used within OpenPNP. A future PR could use the ContactProbingNozzle to calibrate part heights on the fly (first use) to improve precision in bottom vision (perfect focal plane) and in placement.

Implementation Details

  1. This has been tested on my machine with the exception of the pump Actuator (my machine runs the vacuum on a hysteresis instead).
  2. I did follow the coding style.
  3. The spi Head was expanded to get the pump actuator. The spi Nozzle has changed valve methods.
  4. It passed mvn test.

…e.isPartOff().

ReferencePnpJobProcessor.Place.checkPartOff() currently uses Nozzle.pick() and Nozzle.place() for valve switching.

https://github.com/openpnp/openpnp/blob/fee9728215b6174da838a636539370c401a2287c/src/main/java/org/openpnp/machine/reference/ReferencePnpJobProcessor.java#L781-L790

This assumes that PICK_COMMAND / PLACE_COMMAND only contains G-Code to actuate the valve. That may not be true for users that add additional G-Code under the assumption that a “pick” really is a “pick” and not just a valve actuation.

E.g. my machine performs a Z-probe, valve actuation, dwell, slow (relative) retraction sequence there. I guess other users might have similarly elaborate pick and/or place commands.

This proposal actuates the valve inside Nozzle.isPartOff() using the already existing vacuum actuator (currently only used for reading) for switching the valve independently. It uses the ACTUATE_BOOLEAN_COMMAND in order to switch the valve. This addition is also useful for the user to experiment with the valve and vacuum levels, using the Actuators tab and buttons in the Machine Controls.

NOTE: For the Nozzle.pick() and Nozzle.place() the valve still needs to be actuated using the PICK_COMMAND / PLACE_COMMAND G-Code as it may be embedded in the middle of the G-Code sequence. In other words Nozzle.pick() and Nozzle.place() do not actuate the Actuator.
* New spi.Nozzle moveToPickLocation() and moveToPlacementLocation() methods for override in subclasses.
* ContactProbeNozzle overrides these to add contact probing on the Z down move.
* The probe is a boolean Actuator. True means probing to the contact, False means probing away.
* Vacuum valve can be actuated using Actuator.
* Driver pick() place() and PICK_COMMAND and PLACE_COMMAND could be removed in the future. For now I left it in, as I'm not sure how other drivers (Neoden4) can handle it.
* isPartOn() and isPartOff() are enabled individually, if the vacuum level thresholds are set. So users can skip the isPartOff() check to safe time.
@markmaker
Copy link
Collaborator Author

Hi @vonnieda

As you see I tried avoiding mixing in Z-probing per se. If Z-probing using the nozzle is later needed, it can be added on top, as an implementation of the ZProbe interface you discussed.

I left the driver pick() and place() intact, including the PICK_COMMAND/PLACE_COMMAND G-Code fragments, so a transition is possible. Also I don't know how other drivers will handle this (e.g. Neoden4). I guess it could be difficult for them to implement this with generic actuators.

_Mark

@vonnieda
Copy link
Member

@markmaker I'm not comfortable with the dilution of the Nozzle interface here. Please see #855 (comment) for how this can be implemented. As I said before, I don't want to add "another way" to do this. We can choose a way that works and go with that, and users can migrate their settings.

In short, the changes should look quite a lot like:

  1. Remove ReferenceDriver.pick() and ReferenceDriver.place(), and all that goes along with them.
  2. Add configuration for the vacuum actuator into ReferenceNozzle.
  3. Move the pump on/off logic from GcodeDriver to an actuator on ReferenceHead.
  4. Change Nozzle.pick() and Nozzle.place() to both take the Location that the action should happen at.

I believe this covers all known machine configurations for types of pick and place actions, and it keeps the interface semantically clear.

@markmaker
Copy link
Collaborator Author

Hi @vonnieda

Thanks. I see I must have misunderstood the turn things took with the Z probing discussion. I had the impression, the probing motion needed to be extracted from pick(), place(). My mistake. I still don't really see how ZProbe will integrate with the subclass, but I trust you have a solution in mind. :-)

Everything was crystal clear back at the #855 (comment) however, so I will proceed as you suggest. Though not any time soon ... the weekend is over.

I've since seen how drivers other than the GCodeDriver (e.g. the NeoDen4Driver) handle generic Actuators. They just seem to interpret reserved names for Actuators in order to assign correct roles and trigger the right propriatory protocol (or whatever they need to do).

_Mark

@vonnieda
Copy link
Member

I still don't really see how ZProbe will integrate with the subclass, but I trust you have a solution in mind. :-)

Hi @markmaker, are you referring to the ZProbe feature in general, or with regards to your contact probing nozzle? If the latter - I don't think they are related per se. In other words, I don't think the contact probe nozzle would have to use a ZProbe.

The ZProbe feature will just be a new HeadMountable class, unrelated to nozzle or anything else. It will be up to concrete implementations of the ZProbe interface to determine how it works.

I've since seen how drivers other than the GCodeDriver (e.g. the NeoDen4Driver) handle generic Actuators. They just seem to interpret reserved names for Actuators in order to assign correct roles and trigger the right propriatory protocol (or whatever they need to do).

Yes, this is one way to use the ReferenceActuator without dealing with implementing a new subclass specific to the driver. A more "correct" way would be to have an e.g. Neoden4Actuator and it would carry whatever information it needed to do it's job, but hardcoding actuator names is a quick a dirty way to get it done.

Jason

@markmaker
Copy link
Collaborator Author

are you referring to the ZProbe feature in general, or with regards to your contact probing nozzle?

The latter. We spoke about it, you said:

So, I think there will be an interface called Zprobe and I will write one or two concrete implementations. The first will be ActuatorZprobe which will work like what I have now. The next would be VacuumZprobe, and for this you would configure a Nozzle, NozzleTip, and maybe vacuum levels. Users will choose an implementation like they do now for Nozzles, Cameras, etc. This would allow you to write a StereoscopicZprobe, too, of course.

I must have misunderstood that. I thought it means a Nozzle or Camera subclasse could implement the ZProbe interface on top to provide Z-Probing as a secondary service.

But I guess the right understanding is that the Probe would still be a separate machine model object, just referencing the other object that actually provides the service, e.g. reference the nozzle for vacuum sniffing or contact sensing or reference the camera for stereoscopic capture.

Hope I get it this time.

_Mark

@vonnieda
Copy link
Member

are you referring to the ZProbe feature in general, or with regards to your contact probing nozzle?

The latter. We spoke about it, you said:

So, I think there will be an interface called Zprobe and I will write one or two concrete implementations. The first will be ActuatorZprobe which will work like what I have now. The next would be VacuumZprobe, and for this you would configure a Nozzle, NozzleTip, and maybe vacuum levels. Users will choose an implementation like they do now for Nozzles, Cameras, etc. This would allow you to write a StereoscopicZprobe, too, of course.

I must have misunderstood that. I thought it means a Nozzle or Camera subclasse could implement the ZProbe interface on top to provide Z-Probing as a secondary service.

But I guess the right understanding is that the Probe would still be a separate machine model object, just referencing the other object that actually provides the service, e.g. reference the nozzle for vacuum sniffing or contact sensing or reference the camera for stereoscopic capture.

Yes, this is correct. ZProbe will be a "first class" interface, like Nozzle, Camera, etc. Not an extension of those. This keeps the interface simple and extensible.

Hope I get it this time.

_Mark

@markmaker
Copy link
Collaborator Author

Yes, this is one way to use the ReferenceActuator without dealing with implementing a new subclass specific to the driver. A more "correct" way would be to have an e.g. Neoden4Actuator and it would carry whatever information it needed to do it's job...

Well, the more I think about it, the more I'm convinced that the semantics of an actuator is quite well-defined by it's assignment. If you take the vacuum sensing actuator assigned to Nozzle 1, it is actually quite clear what it represents and what's it supposed to do.

We just need to be careful to assign actuators to machine model objects in a semantically unambiguous way.

_Mark

…uators.

* removed pick() and place() methods from the drivers.
* removed GCodeDriver PICK_COMMAND/PLACE_COMMAND fragments.
* Nozzle pick() and place() methods now handle the full action:
   - move and lower the nozzle to the pick/place location
   - actuate the valve Actuator which
   - retract the nozzle after the pick/place
* vacuum pump added to Head, actuated by Actuator
* valve actuation in turn actuates the pump if needed
* removed GCodeDriver PUMP_ON_COMMAND, PUMP_OFF_COMMAND.
* adapted testing to new actuator based picking/placing (as much as I understand)
* added Camera light Actuator
* changed all Wizard Actuator references into ComboBoxes for easy selection
* BUGFIX: machine panels selectedTool notification type cast exception (actuators)

THIS IS WORK IN PROGRESS.

Open Questions:
* How and where to actuate the camera lights with and without settling?
* Should the machine panel selectedTool ComboBox also react to newly created/deleted Actuators?
@markmaker
Copy link
Collaborator Author

markmaker commented Jun 24, 2019

Hi @vonnieda

Note, Travis failure seems to be JDK11 related (it tries to install the JDK and fails, seem not related to the code).
https://travis-ci.org/openpnp/openpnp/jobs/549960857

So this is another try. Tried to follow 1:1 what you suggested. Quite a change!

One known deviation: In pick() I used the feeder as a parameter instead of the location. Why? Because there are feeders that need special motion to approach them. One example was discussed here:
https://groups.google.com/d/msg/openpnp/3GhQhlIK-vw/cD0CnAFxAwAJ
My new feeder that I'm developing will need that too. The idea is to later make it a method of the feeder to move the nozzle over the pick location. I'm very tired, so maybe that's not well described, sorry.

I also changed all the Actuator Name TextEdits into ComboBoxes:
grafik

There is a question with the camera light Actuator. Vision can capture an image with or without settling. The light needs to be ON before settling. Of course I could just duplicate code in settleAndCapture() and in capture(), but it feels wrong and the light would be switched on/off twice with settling. Suggestions?

Had a very hard time debugging the unit tests.

  • How can I force Eclipse to log all exceptions (including stack traces)?
  • How can I get LogUtils.isDebugEnabled() == true in tests?
    (the mvn "Debug Output" option does not help)

This is work in progress. No machine tests so far.

_Mark

@@ -419,8 +419,8 @@ public void mouseClicked(MouseEvent e) {
}
else if (e.getOldValue() != null && e.getNewValue() == null) {
for (int i = 0; i < comboBoxHeadMountable.getItemCount(); i++) {
NozzleItem item = (NozzleItem) comboBoxHeadMountable.getItemAt(i);
if (item.getNozzle() == e.getOldValue()) {
HeadMountableItem item = (HeadMountableItem) comboBoxHeadMountable.getItemAt(i);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is just a bugfix. I think you added Actuators to the dropdown. Now those will create a cast exception.

@vonnieda
Copy link
Member

Hi @vonnieda

Note, Travis failure seems to be JDK11 related (it tries to install the JDK and fails, seem not related to the code).
https://travis-ci.org/openpnp/openpnp/jobs/549960857

Okay, I will look into this.

So this is another try. Tried to follow 1:1 what you suggested. Quite a change!

One known deviation: In pick() I used the feeder as a parameter instead of the location. Why? Because there are feeders that need special motion to approach them. One example was discussed here:
https://groups.google.com/d/msg/openpnp/3GhQhlIK-vw/cD0CnAFxAwAJ
My new feeder that I'm developing will need that too. The idea is to later make it a method of the feeder to move the nozzle over the pick location. I'm very tired, so maybe that's not well described, sorry.

I have some concerns about this, and it interferes with plans I have for part measurement. Two questions:

  1. The thread you linked seems to be related to the user having put something in the way of their home location. The solution is to simply move the home switches, or not put something in the way. Changing the way pick works for one user's misplaced home location is not a good solution.

    The entire point of homing is that we don't know where the machine is yet and we need a reference, so having locations involved in homing is not logical.

  2. Can you describe why your feeder will need a special pick motion? A special feed motion, I understand. That's what feed() is for. And a special post pick motion I also understand, and we have postPick() for that. Both are specific to the feeder. If we truly need another step here, it should be on the feeder, not the nozzle.

This seems to conflate picking with feeding, and I don't want to do that.

I also changed all the Actuator Name TextEdits into ComboBoxes:
grafik

There is a question with the camera light Actuator. Vision can capture an image with or without settling. The light needs to be ON before settling. Of course I could just duplicate code in settleAndCapture() and in capture(), but it feels wrong and the light would be switched on/off twice with settling. Suggestions?

Are you adding light control via actuator here? That seems to be a completely unrelated feature and should be a different PR, I think?

Had a very hard time debugging the unit tests.

* How can I force Eclipse to log all exceptions (including stack traces)?

You can add an exception breakpoint: https://help.eclipse.org/mars/index.jsp?topic=%2Forg.eclipse.jdt.doc.user%2Freference%2Fviews%2Fbreakpoints%2Fref-addexception_viewaction.htm

* How can I get LogUtils.isDebugEnabled() == true in tests?
  (the mvn "Debug Output" option does not help)

Look at the code in the log window that sets the global log level. You can use that in a @Before annotation to set it before the tests run: https://stackoverflow.com/questions/531371/what-are-junit-before-and-test

This is work in progress. No machine tests so far.

_Mark

Before you put further work into this PR let's discuss it and make sure we're both on the same page. I don't want you to waste time just for me to ask you to change everything again.

Thanks,
Jason

@markmaker
Copy link
Collaborator Author

Hi @vonnieda

Thanks for the review.

I have some concerns about this, and it interferes with plans I have for part measurement. Two questions:

  1. The thread you linked ...

Like I said I was tired. :-/ I was just using this example to show that some feeders need to be approached carefully from one side. It has nothing to do with homing, that collision was just the foremost symptom discussed in the group. I was just assuming that - as a generalized case - collisions might also happen when picking parts.

I remember seeing a DIY automatic feeder that fed the tape by the nozzle pushing down on a lever right in the pick (can't seem to find that now). It worked "naturally" with a single nozzle machine. But if the guy had used a multi nozzle machine, moving from one pick location to the next in a straight line would have collided with the feeder mechanics. Yes it can probably be done with Scripting and perhaps some reverse-feeder-for-part finding hack.

Can you describe why your feeder will need a special pick motion? ...

My new feeder is pushing away a cover from the cut strip. The nozzle is tenths of a millimeter above the tape doing that. True, it can be done in the feed operation, at least as long as the processor groups the feed operation with the pick operation on multi-nozzle machines (which is not self-evident, IMHO and should not be taken for granted in a feeder design).

But I really want to keep the nozzle lowered right on the feeder and I do not want to move to safe Z between cover opening and pick. It would actually be safer to do the cover pushing as part of the pick motion for smoothest operation (to avoid vibration and parts to jump out, it's really, really delicate for 0402s). Note this is cheap 3D printed "mechanics", the cover flexes slightly and keeps some tension from being pushed.

Admittedly these are all cheapo DIY examples abusing the nozzle for purposes, you might wrinkle your nose about. Having said that I'm quite sure there are very professional machines out there that need special approach paths to some of their specialty feeders as well.

Ultimately that's why I really think the move to the feeder pick location should be made a method of the feeder for override. Sometimes, nozzle and feeder really need to work together closely and some "conflation" is inevitable. Unfortunately Java has no multimethods :-).

I also think the feeder (of even the PlannedPlacement) should be much more involved and available in the whole placement process (all steps) and generally be avaible in the scripting parameters. Like you said in a related matter;

One thing missing is you can't tell exactly which feeder was used, but if you use one feeder per part you can use the part to find it. If you develop this in a script and find it works well, then we could integrate it further so we know the exact feeder.

Different approaches to providing that are conceivable.

Are you adding light control via actuator here? That seems to be a completely unrelated feature and should be a different PR, I think?

Well, I thought you agreed on this, there:

  1. Do you agree that Actuators should be used for valve and pump? (and camera lights for that matter)

Yes, on all three. Making valve and pump actuators would let us easily move pump control out of GcodeDriver and into Head or Machine, which is where it should be. Camera lights also yes, I will probably take an initial stab at that in the next day or two as I am working on a large set of camera changes.

As it is the same pattern of refactoring things away from GCode and Scripts towards a set of Actuators linked from the right machine model object plus as I was introducing the ComboBox model for Actuators, it seemed the right moment to just take this off your shoulders.

But yes it can be made a separate PR, if you wish.

_Mark

@vonnieda
Copy link
Member

vonnieda commented Jun 25, 2019

Thanks @markmaker, I understand now. Let me think more about this and get back to you soon. I do have a few quick thoughts:

  1. It seems you really actually need something on Feeder, not on Nozzle, right? If you want the feeder to decide how it's approached, it seems that perhaps Feeder.pick(Nozzle) is needed? I am wondering if we can perhaps merge feed, pick, postPick, etc. all into Feeder.pick(Nozzle). This what I'll spend some time thinking about, but feel free to comment on the idea. It requires some thought because this is actually how it used to work a long time ago, and I changed it with presumably good reason, so I need to go revisit those decisions.

  2. Regarding camera lighting: Yes, I agree this is the way to go, but I prefer to have PRs contain as little as possible, especially when new features are introduced. This makes it easier to discuss the feature, reason about it, merge it, and revert it if need be without also reverting unrelated features.

    I expect camera lighting to be a bit of a complex feature, as I'd like to offer control over color and mode, as well as just on and off.

Thanks,
Jason

@markmaker
Copy link
Collaborator Author

markmaker commented Jun 26, 2019

Hi @vonnieda

it seems that perhaps Feeder.pick(Nozzle) is needed

I fear that just reverses the problem, because the Nozzle wants to have its say too. Remember I actually wanted to create a ContactProbeNozzle subclass to create a Liteplacer style nozzle that stops on contact. Both the nozzle and the feeder are instrumental in the pick() step. It's one of these cases where the classical OOP single method dispatch fails to grasp the real world.

https://en.wikipedia.org/wiki/Multiple_dispatch#Examples

In the absence of true multimethods, I think the practical solution is splitting up the pick() and place() into its sub-steps and letting the machine objects most closely associated with each sub-step override the sub-step methods.

A pick() is composed of:

  1. Move to the feeder pick location, first at safe Z, but may in the end involve a special approach path. Does not yet go down in Z (at least not all the way).
  2. Lower the nozzle to the part, may involve contact probing.
  3. Switch on the vacuum valve.
  4. Retract nozzle minimally from feeder, may involve releasing the probe switch.
  5. Move up to safe Z, may at the beginning involve a special retract path.

Consequently I suggest wrapping all this up into the Nozzle.pick() but calling methods feeder.moveToPickLocation(Nozzle) and feeder.retractFromPickLocation(Nozzle) for sub-steps 1 and 5. The default implementation for a feeder is just a moveToAtSafeZ(nozzle, pickLocationAtSafeZ) for the former and a moveToSafeZ() for the latter.

Sub-step 2 would be a protected nozzle method for override in subclasses. It would simply do the final Z down move in the plain nozzle. Override in ContactProbeNozzle with contact probing.

Sub-step 4 would likewise be a protected nozzle method. It might do nothing in the plain nozzle. The override in ContactProbeNozzle would first retract until the probe switch is released. This could also be used to auto-calibrate the true feeder height, so maybe the probing could be limited to the first pick after homing and then proceed at full speed.

The place() is simpler and follows from sub-steps 2 through 4. Likewise, sub-step 4 could auto-calibrate the part height.

If I'm not mistaken, you are strongly motivated to keep the top level interfaces (i.e. Feeder in this case) very, very simple. I agree it makes sense to put up a hard fight before a more complex interface is allowed in. For what it's worth, I would just argue that oversimplification can be as bad or worse than overcomplexity.

What do you think?

Regarding camera lighting...

I'll remove that.

_Mark

@markmaker markmaker changed the title Feature/Contact Probing Nozzles and independant Valve Switching Feature/Actuator based Pick+Place, Contact Probing Nozzle and independant Valve Switching Jun 26, 2019
@markmaker
Copy link
Collaborator Author

markmaker commented Jun 26, 2019

EDIT: I think we should postpone solving this issue. I'll propose a separate PR.


Hi Jason

assuming that this bit is uncontroversial, I looked around a bit and found more Actuator links. Added the ActuatorsComboBoxModel to ReferenceAutoFeeder.

grafik

But now I saw the SwitcherCamera and SwitcherCameraConfigurationWizard. It links Actuators by Id!

It would undoubtedly be better to link Actuators by Id rather than by Name! This could be the opportunity to change that.

Side note: SwitcherCameraConfigurationWizard uses a static combobox i.e. Actuators are not updated, if created/deleted after the Wizard was first opened. Could be fixed by using ActuatorsComboBoxModel, once its Id based.

_Mark

@markmaker
Copy link
Collaborator Author

I edited the proposal post. Please read it online.

_Mark

# Conflicts resolved:
 * src/main/java/org/openpnp/spi/base/AbstractPnpJobProcessor.java
@markmaker
Copy link
Collaborator Author

markmaker commented Jul 2, 2019

Hi @vonnieda

travis fails due to some certificate missing, it seems:
ln: failed to create symbolic link ‘/home/travis/openjdk9/lib/security/cacerts’: No such file or directory

_Mark

* Methods pick() and place() will not do the move to and away from the pick/place location, instead the job processor does it.
* No feeder parameter to pick() needed.
* ContactProbeNozzle does probing in overrides of pick() and place()
* No special Z motion is involved, we simply assume the user to be responsible to capture part/PCB Z heights where the probe is not yet triggered.
* Special feeder approach motion will be solved later.
markmaker added a commit to markmaker/openpnp that referenced this pull request Nov 20, 2019
This is part of openpnp#859. By breaking this into a separate PR I hope to simplify the openpnp#859 diff.
@markmaker
Copy link
Collaborator Author

Hi @vonnieda

The latest commits should resolve remaining concerns about changing the pick() and place() semantics (see point 7 here).

  • Methods pick() and place() will revert to practically the old meaning.
  • They will not do the move to and away from the pick/place location, instead the job processor will do it (as it has in the past).
  • No new feeder parameter to pick() needed. No new location parameter to place().
  • No change in the Nozzle.pick() Nozzle.place() interface.
  • Changes in calling code reverted.
  • ContactProbeNozzle does probing in straight overrides of pick() and place()
  • No special Z motion/positioning is involved "pre-probing", we simply assume the user to capture the part/PCB Z heights slightly above the parts where the probe is not yet in contact/triggered.
  • Special feeder approach motion will be solved later.

If you see "greener light" here, I will go on and test this on the machine, as soon as possible.

Thanks for reviewing.

_Mark

@markmaker
Copy link
Collaborator Author

I've updated the description to reflect the latest state of this PR and to document how to use.

I will now go to the machine and test the latest changes.

@markmaker
Copy link
Collaborator Author

Referenced here:
https://groups.google.com/d/msg/openpnp/763b0vw_PaM/-GRXeiXfBwAJ

A workaround was found, see the updated Description at the top.

@markmaker
Copy link
Collaborator Author

Hi @vonnieda

I have tested everything except the pump on/off governing on my machine.

  1. Using the Actuators freely on the machine controls.
  2. Picking manually on the feeder.
  3. Manual discard.
  4. Runing a mini job, including vacuum sensing, pick and place.
  5. ContactProbingNozzle working as expected.

I cannot reasonably test the pump on/off because my machine runs the pump underpressure on a hysteresis and I have a single nozzle machine, so I cannot test having multiple parts on the nozzles i.e. keeping the pump running as long as needed.

I will report back when the M114 discussion is resolved.

_Mark

@vonnieda
Copy link
Member

Thanks @markmaker. I am working on merging PRs today and this is next on the list. I responded to your M114 question, so let's sort that out and then we'll get to merging.

…des both the "ok" M114 acknowledgment and the position report on one line.
@markmaker
Copy link
Collaborator Author

Pushed the proposed change, so we know what it is about.

1d4b8bd

@markmaker
Copy link
Collaborator Author

I think both M114 issues were resolved, thanks:
https://groups.google.com/d/msg/openpnp/763b0vw_PaM/5jshx_XmBwAJ

It would be nice to be able to merge this.

I'll be offline for the next time. Dinnertime. :-)

_Mark

@vonnieda
Copy link
Member

@markmaker Great work on this Mark! Thank you for patience and your persistence. This is a really nice improvement to OpenPnP.

I am ready to merge this as it is, but I have two asks:

  1. Please add BREAKING CHANGE information to CHANGES.md and explain what users will need to do to migrate their configuration. This will be shown to users when they update and should help ease the pain of migration.
  2. Consider whether it might be worth not removing the GcodeDriver commands so that existing configurations will load without error. I think, as it stands now, if someone upgrades to this they will not be able to start OpenPnP. If you leave the commands in (but not any of the code that uses them) then users could start OpenPnP and copy and paste the commands to the new areas from the UI without having to edit machine.xml by hand.

Thanks,
Jason

@vonnieda
Copy link
Member

@markmaker Nevermind the above. I'm going to do those two things and then merge this.

* Allowed for deprecated GCode fragment migration
@markmaker
Copy link
Collaborator Author

@markmaker Nevermind the above. I'm going to do those two things and then merge this.

Oh, hope we don't do it both! :-)

_Mark

@vonnieda vonnieda merged commit d6c8ce6 into openpnp:develop Nov 24, 2019
@vonnieda
Copy link
Member

Thanks @markmaker!

@markmaker
Copy link
Collaborator Author

@vonnieda

Please feel free to use your own version and/or improve my text. Explaining such complex stuff in English is still veeery hard and I won't feel put off at all if you optimize it :-)

Also I think it would help, if you were to quickly announce the breaking changes in person on the list. Thanks a lot for this great project and the great way you are leading it!

Bed time for me...

_Mark

tonyluken added a commit to tonyluken/openpnp that referenced this pull request Dec 8, 2019
* Switch the vacuum valve independently using the actuator inside Nozzle.isPartOff().

ReferencePnpJobProcessor.Place.checkPartOff() currently uses Nozzle.pick() and Nozzle.place() for valve switching.

https://github.com/openpnp/openpnp/blob/fee9728215b6174da838a636539370c401a2287c/src/main/java/org/openpnp/machine/reference/ReferencePnpJobProcessor.java#L781-L790

This assumes that PICK_COMMAND / PLACE_COMMAND only contains G-Code to actuate the valve. That may not be true for users that add additional G-Code under the assumption that a “pick” really is a “pick” and not just a valve actuation.

E.g. my machine performs a Z-probe, valve actuation, dwell, slow (relative) retraction sequence there. I guess other users might have similarly elaborate pick and/or place commands.

This proposal actuates the valve inside Nozzle.isPartOff() using the already existing vacuum actuator (currently only used for reading) for switching the valve independently. It uses the ACTUATE_BOOLEAN_COMMAND in order to switch the valve. This addition is also useful for the user to experiment with the valve and vacuum levels, using the Actuators tab and buttons in the Machine Controls.

NOTE: For the Nozzle.pick() and Nozzle.place() the valve still needs to be actuated using the PICK_COMMAND / PLACE_COMMAND G-Code as it may be embedded in the middle of the G-Code sequence. In other words Nozzle.pick() and Nozzle.place() do not actuate the Actuator.

* Cosmetic change to use the local instead of member variable.

* Created new ContactProbeNozzle subclass and ContactProbeNozzleWizard.

* New spi.Nozzle moveToPickLocation() and moveToPlacementLocation() methods for override in subclasses.
* ContactProbeNozzle overrides these to add contact probing on the Z down move.
* The probe is a boolean Actuator. True means probing to the contact, False means probing away.
* Vacuum valve can be actuated using Actuator.
* Driver pick() place() and PICK_COMMAND and PLACE_COMMAND could be removed in the future. For now I left it in, as I'm not sure how other drivers (Neoden4) can handle it.
* isPartOn() and isPartOff() are enabled individually, if the vacuum level thresholds are set. So users can skip the isPartOff() check to safe time.

* BREAKING CHANGES: Vacuum Valve and Pump, Z-Probing, Camera Lights Actuators.
* removed pick() and place() methods from the drivers.
* removed GCodeDriver PICK_COMMAND/PLACE_COMMAND fragments.
* Nozzle pick() and place() methods now handle the full action:
   - move and lower the nozzle to the pick/place location
   - actuate the valve Actuator which
   - retract the nozzle after the pick/place
* vacuum pump added to Head, actuated by Actuator
* valve actuation in turn actuates the pump if needed
* removed GCodeDriver PUMP_ON_COMMAND, PUMP_OFF_COMMAND.
* adapted testing to new actuator based picking/placing (as much as I understand)
* added Camera light Actuator
* changed all Wizard Actuator references into ComboBoxes for easy selection
* BUGFIX: machine panels selectedTool notification type cast exception (actuators)

THIS IS WORK IN PROGRESS.

Open Questions:
* How and where to actuate the camera lights with and without settling?
* Should the machine panel selectedTool ComboBox also react to newly created/deleted Actuators?

* * mvn test and bugfixes.

* Reverted the camera light actuator

* Some cleanup and added ActuatorsComboBoxModel to ReferenceAutoFeeder.

* removed unnecessary cast

* Enhance Add pipeline with scaling on each stage

* Simple text changes to address things I found ambiguous when starting
with OpenPnP

* Simple text changes to address things I found ambiguous when starting
with OpenPnP

* Show the limit being crossed

* Reverting this change so I can have a base branch

* git is hard

* Adds scripting event Job.Placement.Starting

* Missed change

* Add BeforeDiscard event

* Added scripting support to discards made via GUI

* Added pipeline stage to write actuators

Signed-off-by: Jarosław Karwik <jkpublic@kartech.biz>

* Update GcodeDriver.java

Add MOVE_TO_COMPLETE_COMMAND to separate the move command from the wait for completion command.  Allows simultaneous moves across multiple drivers.

* Bottom fiducial check fix

Possible solution to bottom fiducial check bugs 819 and 904

* Update ReferenceFiducialLocator.java

Clean up implementation.

* Update ReferenceFiducialLocator.java

Removed unused logger import

* Correction

Removed logger from wrong file

* Update ReferenceFiducialLocator.java

Fixes panel fiducial check for bottom side boards.

* Fixes openpnp#909 / openpnp#911 by using JTable.getRowCount() rather than TableModel.getRowCount(). The former takes filters into consideration while the latter always returns the total number of rows in the model.

* Create FUNDING.yml

* getting rid of toolchanger config backward compatibility due to issue openpnp#913

* Remove duplicated vision offset correction

getPickLocation() is using visionLocation value to calculate pick location. visionLocation already contains value that was previously corrected in updateVisionOffsets().

* Restored old pick() and place() semantics:
* Methods pick() and place() will not do the move to and away from the pick/place location, instead the job processor does it.
* No feeder parameter to pick() needed.
* ContactProbeNozzle does probing in overrides of pick() and place()
* No special Z motion is involved, we simply assume the user to be responsible to capture part/PCB Z heights where the probe is not yet triggered.
* Special feeder approach motion will be solved later.

* BREAKING CHANGE: removing legacy drivers
This is part of openpnp#859. By breaking this into a separate PR I hope to simplify the openpnp#859 diff.

* Removed type cast error in machine controls "nozzles" change listener.

* Reverted some OpenPNP-formatter whitespace artefacts to simplify diff.

* Added support for Smoothieware position reporting. Smoothieware includes both the "ok" M114 acknowledgment and the position report on one line.

* Remove visionOffsets from ReferenceStripFeeder as it's no longer used.

* * Added CHANGES.md entry
* Allowed for deprecated GCode fragment migration

* Minor copy updates in CHANGES and addition of ReferenceStripFeeder update.

* Renamed vacuum sense actuator to vacuum actuator to better reflect it's dual role.
Changed the name of the Part Detection panel to Vacuum, since that's all it is.

* Add note about backwards compatibility.

* MaskHsv can alternatively create true binary masks to recognize any color except the masking color e.g. even black parts can be recognized in front of a green background.

* Add method to determine whether a HeadMountable location is inside soft-limits.

* * divide Lengths
* derive and unit-adapt another partial Location to one
* bugfix in Ransac if the indicesOnLine turns out empty

* Spelling error in comment.

* New vision stages:
* stages/CreateShapeTemplateImage.java
* stages/FitEllipseContours.java
* stages/DrawEllipses.java

* Add Marek's SizeCheck stage: https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/openpnp/PoWL_jL_O_Y/D4WJEDj0AQAJ

* Update LocationButtonsPanel.java

Add scripting event after camera move to position.

* Add Camera Position scripting

Add Camera.AfterPosition scripting for  all "Position the Camera" buttons.

* correct error

change globals.put("camera", this) to globals.put("camera", camera)

* Add ReferenceLeverFeeder

ReferenceLeverFeeder supports feeders where a lever is pushed to feed the parts (such as Alex Feeder).  Made from ReferenceDragFeeder.

* Update CHANGES.md

Add ReferenceLeverFeeder to change list.
tonyluken added a commit to tonyluken/openpnp that referenced this pull request Dec 12, 2019
* Add latest changes to my forked repository (#2)

* Enhance Add pipeline with scaling on each stage

* Simple text changes to address things I found ambiguous when starting
with OpenPnP

* Simple text changes to address things I found ambiguous when starting
with OpenPnP

* Show the limit being crossed

* Reverting this change so I can have a base branch

* git is hard

* Adds scripting event Job.Placement.Starting

* Missed change

* Add BeforeDiscard event

* Allow histogram equalization on color images.

* Add Contrast Limited Adaptive Histogram Equalization (CLAHE)

Added AdaptiveHistogramEqualize stage to the pipeline which utilizes
CLAHE to enhance image contrast.

* Pull in latest changes from openpnp (#3)

* Switch the vacuum valve independently using the actuator inside Nozzle.isPartOff().

ReferencePnpJobProcessor.Place.checkPartOff() currently uses Nozzle.pick() and Nozzle.place() for valve switching.

https://github.com/openpnp/openpnp/blob/fee9728215b6174da838a636539370c401a2287c/src/main/java/org/openpnp/machine/reference/ReferencePnpJobProcessor.java#L781-L790

This assumes that PICK_COMMAND / PLACE_COMMAND only contains G-Code to actuate the valve. That may not be true for users that add additional G-Code under the assumption that a “pick” really is a “pick” and not just a valve actuation.

E.g. my machine performs a Z-probe, valve actuation, dwell, slow (relative) retraction sequence there. I guess other users might have similarly elaborate pick and/or place commands.

This proposal actuates the valve inside Nozzle.isPartOff() using the already existing vacuum actuator (currently only used for reading) for switching the valve independently. It uses the ACTUATE_BOOLEAN_COMMAND in order to switch the valve. This addition is also useful for the user to experiment with the valve and vacuum levels, using the Actuators tab and buttons in the Machine Controls.

NOTE: For the Nozzle.pick() and Nozzle.place() the valve still needs to be actuated using the PICK_COMMAND / PLACE_COMMAND G-Code as it may be embedded in the middle of the G-Code sequence. In other words Nozzle.pick() and Nozzle.place() do not actuate the Actuator.

* Cosmetic change to use the local instead of member variable.

* Created new ContactProbeNozzle subclass and ContactProbeNozzleWizard.

* New spi.Nozzle moveToPickLocation() and moveToPlacementLocation() methods for override in subclasses.
* ContactProbeNozzle overrides these to add contact probing on the Z down move.
* The probe is a boolean Actuator. True means probing to the contact, False means probing away.
* Vacuum valve can be actuated using Actuator.
* Driver pick() place() and PICK_COMMAND and PLACE_COMMAND could be removed in the future. For now I left it in, as I'm not sure how other drivers (Neoden4) can handle it.
* isPartOn() and isPartOff() are enabled individually, if the vacuum level thresholds are set. So users can skip the isPartOff() check to safe time.

* BREAKING CHANGES: Vacuum Valve and Pump, Z-Probing, Camera Lights Actuators.
* removed pick() and place() methods from the drivers.
* removed GCodeDriver PICK_COMMAND/PLACE_COMMAND fragments.
* Nozzle pick() and place() methods now handle the full action:
   - move and lower the nozzle to the pick/place location
   - actuate the valve Actuator which
   - retract the nozzle after the pick/place
* vacuum pump added to Head, actuated by Actuator
* valve actuation in turn actuates the pump if needed
* removed GCodeDriver PUMP_ON_COMMAND, PUMP_OFF_COMMAND.
* adapted testing to new actuator based picking/placing (as much as I understand)
* added Camera light Actuator
* changed all Wizard Actuator references into ComboBoxes for easy selection
* BUGFIX: machine panels selectedTool notification type cast exception (actuators)

THIS IS WORK IN PROGRESS.

Open Questions:
* How and where to actuate the camera lights with and without settling?
* Should the machine panel selectedTool ComboBox also react to newly created/deleted Actuators?

* * mvn test and bugfixes.

* Reverted the camera light actuator

* Some cleanup and added ActuatorsComboBoxModel to ReferenceAutoFeeder.

* removed unnecessary cast

* Enhance Add pipeline with scaling on each stage

* Simple text changes to address things I found ambiguous when starting
with OpenPnP

* Simple text changes to address things I found ambiguous when starting
with OpenPnP

* Show the limit being crossed

* Reverting this change so I can have a base branch

* git is hard

* Adds scripting event Job.Placement.Starting

* Missed change

* Add BeforeDiscard event

* Added scripting support to discards made via GUI

* Added pipeline stage to write actuators

Signed-off-by: Jarosław Karwik <jkpublic@kartech.biz>

* Update GcodeDriver.java

Add MOVE_TO_COMPLETE_COMMAND to separate the move command from the wait for completion command.  Allows simultaneous moves across multiple drivers.

* Bottom fiducial check fix

Possible solution to bottom fiducial check bugs 819 and 904

* Update ReferenceFiducialLocator.java

Clean up implementation.

* Update ReferenceFiducialLocator.java

Removed unused logger import

* Correction

Removed logger from wrong file

* Update ReferenceFiducialLocator.java

Fixes panel fiducial check for bottom side boards.

* Fixes openpnp#909 / openpnp#911 by using JTable.getRowCount() rather than TableModel.getRowCount(). The former takes filters into consideration while the latter always returns the total number of rows in the model.

* Create FUNDING.yml

* getting rid of toolchanger config backward compatibility due to issue openpnp#913

* Remove duplicated vision offset correction

getPickLocation() is using visionLocation value to calculate pick location. visionLocation already contains value that was previously corrected in updateVisionOffsets().

* Restored old pick() and place() semantics:
* Methods pick() and place() will not do the move to and away from the pick/place location, instead the job processor does it.
* No feeder parameter to pick() needed.
* ContactProbeNozzle does probing in overrides of pick() and place()
* No special Z motion is involved, we simply assume the user to be responsible to capture part/PCB Z heights where the probe is not yet triggered.
* Special feeder approach motion will be solved later.

* BREAKING CHANGE: removing legacy drivers
This is part of openpnp#859. By breaking this into a separate PR I hope to simplify the openpnp#859 diff.

* Removed type cast error in machine controls "nozzles" change listener.

* Reverted some OpenPNP-formatter whitespace artefacts to simplify diff.

* Added support for Smoothieware position reporting. Smoothieware includes both the "ok" M114 acknowledgment and the position report on one line.

* Remove visionOffsets from ReferenceStripFeeder as it's no longer used.

* * Added CHANGES.md entry
* Allowed for deprecated GCode fragment migration

* Minor copy updates in CHANGES and addition of ReferenceStripFeeder update.

* Renamed vacuum sense actuator to vacuum actuator to better reflect it's dual role.
Changed the name of the Part Detection panel to Vacuum, since that's all it is.

* Add note about backwards compatibility.

* MaskHsv can alternatively create true binary masks to recognize any color except the masking color e.g. even black parts can be recognized in front of a green background.

* Add method to determine whether a HeadMountable location is inside soft-limits.

* * divide Lengths
* derive and unit-adapt another partial Location to one
* bugfix in Ransac if the indicesOnLine turns out empty

* Spelling error in comment.

* New vision stages:
* stages/CreateShapeTemplateImage.java
* stages/FitEllipseContours.java
* stages/DrawEllipses.java

* Add Marek's SizeCheck stage: https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/openpnp/PoWL_jL_O_Y/D4WJEDj0AQAJ

* Update LocationButtonsPanel.java

Add scripting event after camera move to position.

* Add Camera Position scripting

Add Camera.AfterPosition scripting for  all "Position the Camera" buttons.

* correct error

change globals.put("camera", this) to globals.put("camera", camera)

* Add ReferenceLeverFeeder

ReferenceLeverFeeder supports feeders where a lever is pushed to feed the parts (such as Alex Feeder).  Made from ReferenceDragFeeder.

* Update CHANGES.md

Add ReferenceLeverFeeder to change list.

* Allow pipeline stage ImageCapture to perform averaging

* Changed name of stage from AdaptiveHistogramEqualization to
HistogramEqualizationAdaptive

* Updated ImageCapture to use floating point while averaging.
@markmaker
Copy link
Collaborator Author

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.

None yet

2 participants