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

Pinecil v2 short detection at boot #1729

Merged
merged 15 commits into from Aug 4, 2023
Merged

Pinecil v2 short detection at boot #1729

merged 15 commits into from Aug 4, 2023

Conversation

alextrical
Copy link
Contributor

  • Please check if the PR fulfills these requirements
  • The changes have been tested locally
  • There are no breaking changes
  • What kind of change does this PR introduce?

Bug fix/Feature
If tip is shorted at boot, go into an infinite NOP loop to prevent the FW from running, with the intent to prevent the MOSFET from being damaged

  • What is the current behavior?

If a T12 tip is inserted into the handle too deep, it will short out terminals A & B causing the MOSFET to burn out

  • What is the new behavior (if this is a feature change)?
    If a short is detected between terminals A & B prevent the FW from running and causing damage to the Pinecil v2 Hardware

  • Other information:
    *Short of Pinecil V2 performed with a 1A fast blow fuse, and a 5v PSU to minimize chances of damage to hardware while testing

If tip is shorted at boot, prevent the FW from running, with the intent to prevent the MOSFET from being damaged
@discip discip requested a review from Ralim June 30, 2023 18:45
@discip discip enabled auto-merge June 30, 2023 18:45
@discip
Copy link
Collaborator

discip commented Jun 30, 2023

@alextrical
Is it possible to display a message / warning during the loop, or are we too early for that at this point?

@alextrical
Copy link
Contributor Author

It looks like the main screen is loaded at the time the check is performed, so the screen is initiated. (It shows the iron icon and voltage) assuming there is a messaging system that can be called, then it would be possible to do. Though I'm still new to this project, I compiled it for the first time today, but will see if I can work something out from the notes tomorrow

@discip
Copy link
Collaborator

discip commented Jun 30, 2023

@alextrical
You would need to add something like this:

"WarningThermalRunaway": {
"message": "Thermal\nRunaway"
},

e.g.:

        "WarningShortedTip": {
            "message": "Wrong\ntip!"
        },

Or even suggest the solution:

        "WarningShortedTip": {
            "message": "Please\nchange tip!"
        },

@alextrical
Copy link
Contributor Author

Ideal example, will get that added into the PR tomorrow. That definitely helps clear up what happend to the end user, rather than just locking up

@discip
Copy link
Collaborator

discip commented Jun 30, 2023

I think you know that my example is just a snippet of what needs to be changed:
These lines need to be added to every single translation_xx.json and Translation.h would need to be adapted as well as translations_definitions.json.

Here is an example for what needs to be changed by reference to 'CJCCalibrationDone'.

Pay attention to the order of the items, which must match in all affected files!

I suggest putting it to the warnings. E.g.: after WarningThermalRunaway.

@alextrical
Copy link
Contributor Author

Yep, I noticed it was a smaller part of the solution, but is a great example of a functioning error message, that I can use to track down the relevant code to get it working correctly.

Than you for the heads up on the translations files, I will probably start by filling them all with dummy data. And then do a second pass with adding the translations for the error message

@alextrical
Copy link
Contributor Author

Ive managed to get a warning to display, however it seems that the display still refreshes and removes the message from the display at maybe less than 1/10th of a second. I'm not sure how to trap the process in a For/While loop long enough to lock up the device at this stage.

  } else if (reading < 500) {
    // for (;;) /* Tip shorted, wait until reset */
    while (true)
    {
      OLED::clearScreen();
      warnUser(translatedString(Tr->WarningShortedTip), 3 * TICKS_SECOND);
      wait(10000);
      // OLED::refresh();
      __NOP();
    }

@Ralim
Copy link
Owner

Ralim commented Jul 2, 2023

The OLED should only be written to by the GUIThread, deliberately to avoid this issue.
Probably best action to do is to add to bsp.h some sort of function like isTipShorted() that returns a bool.

Then in the gui thread warnings area you can call it to check.

You will need to add a stub that returns false to all the other implementions (Pinecilv1,Miniware,MHP30,S60) for now.
(Technically TS101, TS80(P) can do this iirc but not fully implemented).

@Ralim
Copy link
Owner

Ralim commented Jul 28, 2023

I have updated this to have the translations; @alextrical could you give this a once over / test please 🙇🏼 ?

@Ralim Ralim added this to the 2.22 milestone Jul 28, 2023
@ia
Copy link
Collaborator

ia commented Jul 28, 2023

Aaahh! I just was going to pick this up to help & to work on that too on this weekend, I swear. :D

@discip discip merged commit 4533c2f into Ralim:dev Aug 4, 2023
15 checks passed
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

4 participants