edgetx icon indicating copy to clipboard operation
edgetx copied to clipboard

fix: halt T14 power up when USB connected

Open pfeerick opened this issue 1 year ago • 6 comments

Issue being resolved:

  • as I've commented previously and was just reminded of in a review, T14 will power itself on if the USB is connected to charge, and re-power itself (in EM) if powered off with the USB connected.
  • this will catch power-on via USB for any radio with soft power button, and effectivel hang it until disconnected, and then power off
  • while we're here doin' nothin', flash those leds as a crude "state of charge" indicator. I had initially gone for a simple "on/off" blink, but then though someone would thing the handset was just resetting itself, hence the more elaborate and more heartbeat-like timing.

It's just a PoC, and I'm not particuarly attached to the blinky light code - and if it stays the voltage values probably need tweaking.

@3djc What do you think?

edit: Now that I look at it more, maybe it could be juggled around it is present if the radio has if defined(PWR_BUTTON_PRESS), and then either run the FUNCTION_SWITCH specific, STATUS_LED specific or no status led/no FS code (i.e. just halt)?

pfeerick avatar Feb 16 '24 03:02 pfeerick

we probably need a new define, or use defined(MANUFACTURER_JUMPER) as radio with a dedicated charging port don't need that. Not overly fan of the blinking leds (especially as it means error on some radio), so maybe fixed leds ?

Sorry for having forgotten about that.

Thinking about something simple like

#elif defined(STATUS_LEDS)
      // Use Status LED to indicate battery charge level instead
      if (getBatteryVoltage() <= 660) ledRed();         // low discharge
      else if (getBatteryVoltage() <= 842) ledBlue();   // charging
      else ledGreen();                                  // charging done
#endif

3djc avatar Feb 16 '24 07:02 3djc

dedicated charging port

Ah, so that's why the USB_CHARGE define wasn't making much sense to me... the 'dedicated' is missing :laughing: Should just be able to go with the absence of that then as another condition.

Sure, static leds is good, was mainly only there at first to make sure the loop was still running properly (can't blink if it's not working ;) ).

pfeerick avatar Feb 16 '24 08:02 pfeerick

USB_CHARGE also implies that we are getting some info from it (currently a single digital IO that matches the charging IC led output). So we theoretically could get it also from a radio with single USB port

3djc avatar Feb 16 '24 09:02 3djc

Well, whichever way we do it, it's going to be wrong in the future... either someone other than Jumper will do a combined data+usb port (I know there has already been mention of plans for that) or there will be a combined port that also gives us data?

The question is what is the current state of affairs? None of the other single port radios from what I can see in the various target configs define USB_CHARGER... So I think a define will be needed at some point (or it could be renamed - DEDICATED_USB_CHARGE_WITH_DETECT if you wanna be verbose :zany_face: ) - unless instead verify the absence of USB_CHARGER_GPIO_PIN rather than USB_CHARGER?

pfeerick avatar Feb 16 '24 09:02 pfeerick

You cannot use only USB_CHARGER_GPIO_PIN since you need to include a usb charge 'driver'. We can leave it as currently too, it just means that small piece of code will never be run on radio with 2 ports, that's all

3djc avatar Feb 16 '24 09:02 3djc

Why do you need to include the driver? Instead of !USB_CHARGER wouldn't it be !USB_CHARGER_GPIO_PIN ? Meaning if a single port radio is added that does have charge monitoring, more code will be needed since this would automatically skip (I could be mean and throw in an error ifdef to make sure that happens 😅)

pfeerick avatar Feb 16 '24 10:02 pfeerick