-
Notifications
You must be signed in to change notification settings - Fork 8
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
Image print metrics and advanced printer alarms #41
Conversation
…add metricsThread() plus dependencies
…add missing thread join
Hi @maehw,
Yes, this is a bug. It introduces a memory leak as well as keeping the connections open. Solution would be to either
No, sorry I haven't written the web interface, that was a buddy of mine. Are you sure it's just because of the new browsers, because then maybe recompiling the web interface with an updated version of the web framework will solve it? I also can't see how your changes would break the web interface. Does the master code still run as expected?
I haven't used it in ages, back then we had this manual: Manual_Drucker.pdf
Yes, it takes ages. Even during development on my desktop PC. That's why we got the large printer instead for events.
It should lock the vector which is modified by two threads. If a new HTTP connection is made on each call, no need to lock those, if the connection is shared, it should also have a lock.
Me neither, ChatGPT says:
Yes, you can either use condition variables or semaphores. Semaphore is the way to go here I think.
Nicest would be to migrate to something like spdlog where we can set the log level on runtime.
When using spdlog, file logging with log rotation is free. CSV would be nice for advanced statistics, basically you need to decide whats best for you. I think logging is good enough for analysis.
Yes, after noticing how annoying it can be to change the license to something more liberal, I just added the CLA bot to the
|
Okay, I think this is it regarding my functional changes. I am quite happy with the results. Still need to investigate the web interface issues though - I'll test it on master branch and if it's OK with you, I'd open another separate issue with the findings if it's reproducible on the master branch as well. I've prepared CSV export, but I guess this is something which should not be there by default. Any further thoughts on the PR from your side? |
Looks like I put this there for debugging, but in order to use it, we'd have to instantiate a FileSystemLogger instance in code, currently I can't find a place where it's actually constructed. This would also explain the SEGFAULT during shutdown, the (in my opinion non-existing) FileSystemLogger gets shut down. Integrating spdlog should be easy, I will give it a go after merging this PR.
No, the PR looks good to me as is. Thank you so much for working on this project! |
So the PR is ready to merge from your side (also asked on Discord, not sure where you will see it earlier)? |
Thanks, Clemens. Yes, ready to merge! :) Thanks for accepting this contribution. |
The contributions in this pull request shall help to support #39.
TL;DR:
Every print job submitted by self-o-mat now gets tracked with some timing information (e.g. timestamps of self-o-mat processing start, user confirmation, creation of CUPS print job). Another "logic" thread has been added which monitors the print job after handover to CUPS: when does it get processed by the printer and when is it completed - as well as the job's state. This allows to detect timing anomalies (in the future! e.g. unexpectedly long CUPS processing duration by the printer or duration in the pending state). I've also introduced new alarms when the printer needs attention (printer out of paper, printer out of ink, printer's input tray missing) which should improve self-o-mat operation.
Details
Timestamps, job ID and job state for every CUPS print job submitted by self-o-mat are kept in a list (std::vector of a struct). The struct is initially filled in the already existing printer thread. If the user does not cancel printing respectively does confirm printing, the entry (struct) is added to the list. This marks the handover to the new printer monitoring thread. The latter keeps asking CUPS for the current job state, the timestamps for CUPS job creation, processing and completion - updates the list item accordingly and takes actions depending on the current state. When a job is completed (or no longer visible in CUPS' print jobs, e.g. because it had been removed by the admin manually) - this includes a successful as well as an unsuccessful print (states canceled, aborted, completed), the print job entry gets removed from the list and hence is no longer being monitored (why should it). During removal from the list, the job details are emitted to the log (future idea: use a separate external log file, CSV?).
Unintentional by-catch (feature!)
When checking print jobs in Raspberry Pi's print queue UI manually, I found out that a print job could be queried for its IPP
job-printer-state-reasons
. This way, it is possible to detect actual problems while a print job is being processed (and never finishes!): at least for the SELPHY printer "media-empty-error
" and "media-needed
" indicate that the printer is out of paper, "marker-supply-empty-error
" indicates that the printer has run out of ink and "input-tray-missing
" indicates that there is no paper input tray at all (oops, forgot to put it back in?). I find this extremely helpful when operating self-o-mat, especially as my booth has the printer hidden halfway in the housing and its display cannot be seen by the users. Hence, I implemented a CUPS/IPP query for these job details and emit one out of the three new alarms when such a problem is detected.Summary of changes
PrintManager
acts as the CUPS/IPP interface, I've extended and slightly modified its interface:printImage()
does no longer return a boolean (had not been checked before) but now returns the CUPS job IDgetJobDetails()
a method to query the job state (introduced new typePrinterJobState
internally mapped from CUPS) and the relevant timestampsprinterJobStateToString()
a simple helper method which converts the new magic values of the enum type to a human-readable stringcheckPrinterAttentionFromJob()
: internally queries thejob-printer-state-reasons
attribute for a CUPS job by its ID and returns newly introduced error flags (PrinterAttentionFlag
) which are used by "logic" to emit alarmsBoothLogic
:printerThread()
: collecting timestamps and adding them to metrics struct, eventually stored as list elementsprintMonitoringThread()
which contains the core functionality of print job monitoring and checking for additional printer attention (alarming out of ink, out of paper and missing paper input tray)IGui.h
: added an alarm enum value for printer alerts (I did not want to interfere with the existing alarm sources)Interesting finds
PrinterManager::resumePrinter()
does not callhttpClose(http)
explicitely - is this handled automatically (e.g. when loosing context i.e. exiting the method) or a potential bug? (I haven't tried to track any open HTTP connections.. but maybe it could be done this way)Open questions
PrinterManager
mutex: I was unsure aboutprinterStateMutex
- what should it really lock? Concurrent access to CUPS/IPP or some data structures? I think I need input from your side whether it shall be used in the new methods or not.i18n/fr_frontend.json
: No clue if French people would phrase it like thatTODO
s in the code, print thread could explicitly activate print monitoring thread - and print monitoring thread could periodically activate "itself" until the metrics list runs empty.. and then be activated again from print thread. Can you wait for a signal from another thread with a timeout? (Sorry, only familiar with few RTOSes, not with Boost threads and its signalling. Just saw something called "condition variables" in your code already in use). BTW the chosen period time also influences with what delay alarms may be shown/hidden (obvious when you know it) - this may influence usability/UX.Edit: I think the UI files are missing. This is due to working remotely on the RPi from another machine and with no direct GitHub access on the RPi. I'll add them soon.Fixed