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

Print Estimated Time #341

Merged
merged 5 commits into from Oct 31, 2018
Merged

Print Estimated Time #341

merged 5 commits into from Oct 31, 2018

Conversation

kaalleen
Copy link
Collaborator

This will add the estimated time value.

... but it will need some tweaks. Most of all I got a little confused over color change and stop values.
After thinking it through, it doesn't seem to make a lot of sense to have both values in there, but I wasn't so sure about it, so I left it in anyway.

@wwderw

  1. Do you think color change and stop values should be treated separately or can we get rid of the stop value? Means, is there any difference in time prediction for these two?

    As is: each stop causes a new color block to be created, this means we calculate the stop on top of the color change value - an other option would be to remove the color change time if "stop after" is checked

  2. Do you think the color change time should be added to each color swatch (detailed view) or only to total time?

    Right now it's applied to each of them, but could be easily changed.

  3. How should we threat the last color swatch?

    Since there will be no color change anymore, I removed the addition of the color change time, while the stop time would still be calculated.

I think it could be very confusing for the user if both values (stop and color change) are opposed?

@wwderw
Copy link
Contributor

wwderw commented Oct 19, 2018

  1. No stop values, as the stops have a variable of how long it takes the user to resume the pattern. That will vary too wildly.

  2. I would just do the overall time, I wouldn't worry about the individual color block time. However, if there is a calculation for thread used, I would keep that total and on individual blocks.

  3. Calculate the change between the next to last and last color swatch change, but at the end of the last color swatch, treat it like a stop and just ignore it in the calculations.

@kaalleen
Copy link
Collaborator Author

Stops are gone from the calculations, feels a lot easier now ;)
If you find some free time, could you do some testing?

@wwderw
Copy link
Contributor

wwderw commented Oct 19, 2018

You know, I actually think the time per block is actually growing on me. I'm not used to seeing that, but I think that's pretty good. It could be a way to trouble shoot some color blocks for production time concerns as well.

I did change 1000 spm to 700 spm. 1000 is the max on quite a few machines and some of those same machines limit hat speed well below that (Barudans are at 800 and Brother/Babylock's are at 650).

Otherwise, everything is looking good. Where I would expect it to be on the approval sheet. For the operator view, I'm liking it on each color block as well. That will really help with trouble shooting a file for production as well.

@lexelby
Copy link
Member

lexelby commented Oct 20, 2018

Awesome!!

I had originally assumed the calculation code would be in Python, but it really works well in Javascript. I love how you added the settings pane so that I can adjust speed and times and save them as defaults. Great work!

@lexelby
Copy link
Member

lexelby commented Oct 20, 2018

Can we have a "Show estimated time" checkbox in settings as well? Maybe I don't want to bother customers about the time it'll take me to sew out a design. On the other hand, for a design I'm selling as a DST, maybe I do want to include the time.

Ooh, or maybe even separately allow showing time estimates in client views versus operator views?

Oh, another thing I just noticed. Should we change the estimated time on the client detail view pages to be the estimate for each color block?

@wwderw
Copy link
Contributor

wwderw commented Oct 20, 2018

Can we have a "Show estimated time" checkbox in settings as well? Maybe I don't want to bother customers about the time it'll take me to sew out a design. On the other hand, for a design I'm selling as a DST, maybe I do want to include the time.

That's a good point. Never would have occurred to me as most of my customers are ones that will run the files for their own production (be it stock or custom digitizing).

That may not be something that you want them to know as well.

@kaalleen
Copy link
Collaborator Author

@wwderw glad you like having the time on individual color blocks - I simply forgot to take it out ;)

Since we leave it there and you like it for speed optimization ...
Maybe we actually should not calculate the color change time into the single blocks and have it in addition only to the total time calculation?

@lexelby good idea. Checkboxes are set!

@kaalleen
Copy link
Collaborator Author

I had originally assumed the calculation code would be in Python, but it really works well in Javascript.

This is true for the time calculation. 'thread used' is a completely different story.

@kaalleen
Copy link
Collaborator Author

Now it will only add the time needed for color changes to the total time value. I think this makes sense, because the color change time is somewhat in between the color blocks.

@lexelby ready to merge?

@lexelby
Copy link
Member

lexelby commented Oct 24, 2018

Yup! Except there's some weird build failure... Did you even change base.py?

@kaalleen
Copy link
Collaborator Author

Hm, that's strange. I only changed a very few things with the last commit and only within the print template. But it seems like all the other pull requests have build failures too?

@lexelby
Copy link
Member

lexelby commented Oct 30, 2018

Yup, looks like a new test added by pyflakes or something. It uncovered a latent bug, so that's cool! I've fixed it in #330 and I'll merge the fix in here when I get a chance.

@lexelby lexelby merged commit 98612ee into master Oct 31, 2018
@lexelby lexelby deleted the print-estimated-time branch October 31, 2018 00:06
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

3 participants