techninja / cncserver

A RESTful API server for driving serial based CNC devices
133 stars 40 forks source link

AxiDraw V3 draws at half size #137

Closed ghost closed 1 year ago

ghost commented 1 year ago

Using the default axidraw.ini configuration, the AxiDraw V3 draws at half the expected size. Ie., instructing the pen to move to {x: 100, y: 100}, only moves the pen along half the range of each axis.

I have two potential fixes, but I want to check which approach is preferable before opening a PR.

One approach is to add a new axidraw_v3.ini configuration, as I've done in my fork, and had been about to open a PR for.

The other is to make a two-character change in the existing axidraw.ini - enablemotors = "EM,%p" to enablemotors = "EM,%p,1". It appears without this second parameter (which is required in the latest firmware), the command is ignored by the V3, so it remains in the firmware's default 1/16 precision mode, rather than the 1/8 precision the maxArea width/height is set for, giving the half size drawing behaviour.

However, I don't have access to V2 hardware to check this isn't a breaking change.

In any case, there does appear to be an issue in the axidraw.ini (and axidraw_v3a3.ini) configurations, in that the one-parameter enablemotors command matches the legacy firmware, but the comment (that precision is a value from 1-5, rather than 1-4) matches the current firmware.

oskay commented 1 year ago

Can you say what you are referring to by "V2 hardware"?

ghost commented 1 year ago

Sorry, I mean the AxiDraw V2 there

oskay commented 1 year ago

AxiDraw V2 has the same travel size as AxiDraw V3, and its firmware can be upgraded to the same as the V3 as well.

ghost commented 1 year ago

AxiDraw V2 has the same travel size as AxiDraw V3

Ah, yep, and resolution, which makes me confused how the current axidraw.ini configuration works with the AxiDraw V2 tbh.

If it's running legacy firmware, EM,2 should put it in 1/4 step mode, leading to output being double the expected size.

If it's running non-legacy firmware, EM,2 isn't supported (as it's only one param), and I'd expect it to stay in the firmware default 1/16 step mode (as I've observed with my AxiDraw V3), leading to output being half the expected size.

I can only speculate that a minor firmware version update changed how the non-legacy firmware handles receiving EM with only a single parameter.

I'm fairly reluctant to open a PR to make the change from EM,%p to EM,%p,1 in axidraw.ini when I don't understand how the current configuration is working in the first place, and can't test that it doesn't break anything. Are you in a position to test this change against AxiDraw V2 hardware?

oskay commented 1 year ago

I would say that there's "nothing to test".

(Let me further ask: What problem are you trying to solve? Are you aware of any case where someone has put a legacy-model EBB on an AxiDraw V2?)

ghost commented 1 year ago

The problems I'm trying to solve are:

  1. to get my AxiDraw V3 drawing at the correct size with cncserver
  2. without breaking anything for anyone for whom the current AxiDraw configuration works

I'm working on the assumption that the current axidraw.ini does actually work with the AxiDraw V2.

No, I'm not aware of anyone putting legacy firmware on an AxiDraw V2. But it's noteworthy that the current axidraw.ini is using the single-parameter version of the EM command, which going by the EBB docs is only supported by the legacy firmware.

My point was that, going by the EBB documentation, the current axidraw.ini shouldn't work for the AxiDraw V2, whether it's running legacy or non-legacy firmware. But presumably it does.

Given that I can't reason why the current axidraw.ini works for the AxiDraw V2, I can't safely reason that making a change there won't break things.

Going back to my original post, I'm happy to open a PR with a new axidraw_v3.ini, if that's the preferred fix. Or, if the preferred fix is to change EM,%p to EM,%p,1 in the current axidraw.ini, that's not a PR I'd personally be comfortable making, for the reasons stated above, and I'll just continue working in my fork.

oskay commented 1 year ago

I'm not sure that I'm following.

So far as I know, there isn't any functional difference in the electronics, firmware, or mechanics that can cause a different behavior between the AxiDraw V2 and V3.

As it says in the EBB docs, the legacy version of EM "is only for legacy EBB hardware, v1.1."

So far as I know, no AxiDraw V2 or newer has ever used that control board, so it's not a relevant target here.

ghost commented 1 year ago

So far as I know, there isn't any functional difference in the electronics, firmware, or mechanics that can cause a different behavior between the AxiDraw V2 and V3.

And yet there seems there is a difference, as axidraw.ini doesn't work for my just-purchased AxiDraw V3, but (going by comments and commit history) does for the AxiDraw V2.

As it says in the EBB docs, the legacy version of EM "is only for legacy EBB hardware, v1.1."

And confusingly this is what the current axidraw.ini is using.

I'm going to back out at this point and continue working in my fork. All the best.

oskay commented 1 year ago

IMO, it's fine to completely drop any support for AxiDraw V2, if you think that there's a difference. AxiDraw V2 hasn't been for sale since 2016.

ghost commented 1 year ago

Ok, couldn't let this go, and did a bit more investigating 😛.

Turns out there's an undocumented change in the behaviour of the EM command between 2.7.0 and 2.8.0 of the EBB firmware. In 2.7.0, EM,%p behaves as EM,%p,1. From 2.8.0, EM,%p is ignored. So this explains why axidraw.ini worked for people previously, but not for me.

I've verified that changing EM,%p to EM,%p,1 in axidraw.ini works as expect with firmware as far back as 2.4.3 (the earliest version I could try from here).

Now that I've actually understood what's going on, and done due diligence to check this is unlikely to be a breaking change for anyone, I'm happy to make the two-character change. Will open a PR soon.

Thanks for your help.

EmbeddedMan commented 1 year ago

This is not technically a bug - the command is not being used as the documentation states.

In the command documentation, both parameters are shown as required. Thus, "EM,1" is an invalid syntax for all existing versions of EBBs (the v1.1 board is not currently in use anywhere that I'm aware, thus the legacy version of this command doesn't apply). So by using both parameters (i.e. "EM,1,1") you will always get the expected behavior. By using only one parameter you are using invalid syntax and can't expect the command to work properly.

Now, that's the pedantic view of things. In reality, the EBB firmware itself has the second parameter as 'optional'. (In both 2.7.0 and 2.8.0.) In v2.7.0 and before, this optional nature to the second parameter worked properly. However, in v2.8.0 (and beyond) a bug was introduced during the move of the EM command from 'USB parse time' to 'Motion FIFO execution time'. That bug will cause the EBB to ignore any EM command which does not have both parameters, even though the second one is marked optional. (The bug is in the logic for detecting a missing second parameter.)

We should probably change the EBB behavior for future firmware versions. It seems to me there are two possible ways to handle it:

A) We can make the previous behavior work properly in future versions. So a missing second parameter will not cause the command to be ignored, duplicating the 2.7.0 and before behavior. The docs should then be updated to say that the second parameter is in fact optional.

B) We can make the code match the docs by throwing an error if the second parameter is missing - i.e. make it a required parameter just like the documentation says it should be.,

Any preference?

On Thu, Nov 2, 2023 at 7:22 AM Bryan Gale @.***> wrote:

Ok, couldn't let this go, and did a bit more investigating 😛.

Turns out there's an undocumented change in the behaviour of the EM command between 2.7.0 and 2.8.0 of the EBB firmware. In 2.7.0, EM,%p is behaves as EM,%p,1. From 2.8.0, EM,%p is ignored. So this explains why axidraw.ini worked for people previously, but not for me.

I've verified that changing EM,%p to EM,%p,1 in axidraw.ini works as expect with firmware as far back as 2.4.3 (the earliest version I could try from here https://github.com/evil-mad/EggBot/tree/master/EBB_firmware/EBBUpdater/deprecated ).

Now that I've actually understood what's going on, and done due diligence to check this is unlikely to be a breaking change for anyone, I'm happy to make the two-character change. Will open PR soon.

Thanks for your help.

— Reply to this email directly, view it on GitHub https://github.com/techninja/cncserver/issues/137#issuecomment-1790627863, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADN4CDHW2VMRW3OYJIWBX3YCOF6VAVCNFSM6AAAAAA6WFTVOWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJQGYZDOOBWGM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

oskay commented 1 year ago

Well, Bryan and Brian, It looks as though the docs did say that the second part was optional, at the time that this cncserver code was written: https://github.com/evil-mad/EggBot/commit/f3acc5ede06beb027a98143e443baaf1a2eac332

I recommend that (1) we update the EBB docs to indicate that there was a change to the behavior of EM in 2.8.0 and (2) Update the cncserver ".ini" files -- all of them to add in the second parameter, so that they will work across firmware versions more consistently.

EmbeddedMan commented 1 year ago

No problem. EBB documentation has been updated. Please review and suggest changes.