edgetx icon indicating copy to clipboard operation
edgetx copied to clipboard

Add SF/LS custom names + new "SAFE" LS

Open shane-droid opened this issue 3 years ago • 52 comments

Summary of changes:

Firmware:

  • Adds custom name to MODEL special function
  • Adds custom name to logic switches creates a new logic switch type known as SAFE.
  • friendly name displays on 64 sw view page if name is not null, otherwise displays switch type

Companion:

  • new LS type: SAFE
  • custom friendly name to LS and SF.
  • quality of life: SF retains data for channel override after changing combobox selection. E.g: BEFORE: In SF1, changing CH1 override to CH2 override, will casues all fields for SF1 to reset.
  • switch combo boxes now have LS friendly name as well. Eg LO1 now is L01:friendly Name

BUGS: must Shutdown Radio BEFORE disconnecting usb cable. If editing Global functions in companion & then writing to radio, you must Shutdown Radio BEFORE disconnecting usb cable, otherwise the radio will use its inmemory data to overwrite any deletes or cuts that were made to global funtions. see comment: https://github.com/EdgeTX/edgetx/pull/2085#issuecomment-1280731787

What Safe logic sw does:

Stops accidental physical switch ACTIVATIONS from effecting channel output.

  • The SAFE logic switch state [on/off], can only be changed when the secondary physical switch is in an active position.
  • When the secondary physical switch is off, then the SAFE logical switch is locked in either its Last state.

shane-droid avatar Jun 23 '22 10:06 shane-droid

@pfeerick hi, there was a conflict with src/datastructs.h : is this the correct way to resolve it, ie using 2 commits?

  1. merged main to my branch
  2. added a resoultion for the conflict?

shane-droid avatar Jul 01 '22 10:07 shane-droid

Something is still wrong here as compilation broke, and there is a change to libopenui which probably shouldn't have happened. Leave it with me for a day or two and I should be able to sort it out, and review this for merge also. This will still need a little more work, as Companion also needs to at minimum knows about the custom labels, otherwise it will obliterate them when the radio is next synced. And obviously/preferably, allow you to edit them lol. As well as the new LS type. It would also be nice to have on B&W, but I'm ok with it being a colorlcd feature to start with.

But to answer your question, since this will probably need to be squash merged, that should be fine. But normally if there was a conflict due to something in the base branch having changed, you would rebase your branch on the current state of main, and resolve the conflict when re-applying your commits. While this is 're-writing history', it has the effect of also keeping the commit history clean.

pfeerick avatar Jul 02 '22 07:07 pfeerick

@pfeerick ok thanks, i'll have a look at companion

shane-droid avatar Jul 03 '22 00:07 shane-droid

I've rebased this against the current state of main, which has removed the libopenui conflict (as that is now in sync again), and also updated the datastructs as the current state would have reverted a change made in main due to another PR being merged. Next will be functional tests.

pfeerick avatar Jul 06 '22 01:07 pfeerick

Companion side - will you work on it?

Eldenroot avatar Aug 20 '22 05:08 Eldenroot

@pfeerick how can I help further?

shane-droid avatar Sep 24 '22 02:09 shane-droid

Once 2.8 is out of the way this will be one of the PRs that will start to work on... it just kept being put on the back-burner due to bughunting and fixes.

pfeerick avatar Sep 27 '22 06:09 pfeerick

Cool :) 2.9 final is planned when? In 3 months?

Eldenroot avatar Sep 27 '22 07:09 Eldenroot

Yes, pretty much.

pfeerick avatar Sep 27 '22 08:09 pfeerick

A lot of work has been done on this... recent commits look nice and it is moving to the final stage :) I hope we will see it in 2.8.1 :D

Eldenroot avatar Oct 08 '22 06:10 Eldenroot

@eldenroot : its a while away, because the guys are still working on second half of the ui update. and it hasn't been tested.

If you have any ideas on how to: In companion, put LS names on the radio outputs widget, that would be great: image

This crashes the simulator because, {i think, } radio data is null.

https://github.com/EdgeTX/edgetx/pull/2085/commits/21fc7a1f212a9f03e4cf87e24abbd0e142a3977c

image

shane-droid avatar Oct 08 '22 12:10 shane-droid

@shane-droid please limit the PR to its initial scope ie changes to mixes should be based on an Issue and submitted as a separate PR to address the mix Issue. This makes reviewing, accepting, rebasing and merging so much easier especially with so much activity in this project. Also all proposed changes may not be accepted by the reviewers/community and the PR would need to be spilt.

elecpower avatar Oct 10 '22 10:10 elecpower

@elecpower understood. Which branch should I base the changes to mixes on, in order to sub another PR?

shane-droid avatar Oct 10 '22 21:10 shane-droid

@shane-droid main should always be your base otherwise things can get ugly for all those concerned

elecpower avatar Oct 11 '22 03:10 elecpower

BUGS: Introduces a bug in companion. Global functions Cut and Delete operations are affected. They appear to work in companion, but once the model is loaded to the Radio, the changes haven't persisted.

shane-droid avatar Oct 15 '22 08:10 shane-droid

Update on bug hunt: Appears to be realted to radio firmware: USB connection mode. In companion,

  1. editing Global functions with a delete or a cut operation,
  2. then writing changes to Radio, producess the correct radio.yml on the SDcard in the radio.

The bug arises when the USB cable is disconected, The radio.yml file is overwritten with data from the radio's memory, and so the deleted items are repopulated. However, other change operations such as update a global function [ i.e. changing switch SA- to SB- work as normal.]

The Temporary solution is: after editing model in companion and wrting to the radio, Then turn off the radio BEFORE disconnecting the USB cable, then restart radio.

Note: This does not happen in companions simulator. Only live firmware.

Unfortunately, I don't have the knowledge to debug the firmware.

shane-droid avatar Oct 17 '22 11:10 shane-droid

@shane-droid I've rebased this to cleanup the commit history (i.e. now it only shows the commits unique to this PR) and also made some minor corrections (yaml, RadioData and ModelData sizes, as well as some changes needed for LVGL) so hopefully everything compiles ok (I only did a couple local builds) and then we can start reviewing this and figure out any bugs lurking. Then there'll be a call for translators once it's ready.

edit: Hm, the changes in conversion will need to go... this is a LS that doesn't exist in older version of EdgeTX, and more specifically, OpenTX, so that should not be touched.

pfeerick avatar Dec 07 '22 06:12 pfeerick

@pfeerick The "AND" switch hasn't been programmed with any functionality for LS SAFE function [ works as normal for other LS functions ]. I left the "AND" switch in the GUI for extensibility. There is also the bug in companion for saving data. See comment https://github.com/EdgeTX/edgetx/pull/2085#issuecomment-1280731787

shane-droid avatar Dec 08 '22 00:12 shane-droid

@pfeerick The "AND" switch hasn't been programmed with any functionality for LS SAFE function [ works as normal for other LS functions ]. I left the "AND" switch in the GUI for extensibility. There is also the bug in companion for saving data. See comment #2085 (comment)

Shane-droid - I cannot test it because it is not merged and bug in companion. Anyway I just wanna be sure - can I use it for example as a motor emergency switch?

Eldenroot avatar Jan 01 '23 09:01 Eldenroot

I cannot test it because it is not merged

You don't need PRs to be merged to test them - downloadable assets are available for ALL PRs for 14 days after the last commit. This PR needs rebasing before it can be revisited, which will happen again soon.

IIRC, this was more able being able to have a two-stage system, whereby in the first stage the switches could be flicked to get the voice prompts on what they did, and on the second stage they would actually carry out the configured function. For a 'motor kill' you would use the conventional override SF, or a mixer replace line.

pfeerick avatar Jan 01 '23 09:01 pfeerick

OK, thx. Did not know that.

I will wait, need to understand this feature better and try it on my own :)

Thank you

Eldenroot avatar Jan 01 '23 09:01 Eldenroot

  1. What is max data length of
  • custom name for Logical Switch
  • custom name for Special Function
  1. Are there screenshots of how this custom name display were initialy implemented in UI?

JimB40 avatar Jan 12 '23 14:01 JimB40

@Eldenroot yes, it can be used as a motor cut off. UNDERSTAND that it requires two switches to do any task. One switch for the action, and a second switch to confirm the action

shane-droid avatar Jan 12 '23 22:01 shane-droid

@JimB40 I think the Max char number for both is set in data struts file, I think the value is set at 10. I'm away on hols atm.

shane-droid avatar Jan 12 '23 22:01 shane-droid

@JimB40 radio/src/dataconstants.h. Line 139 and line 140.shows them as being 10, under my pull req

shane-droid avatar Jan 12 '23 22:01 shane-droid

@JimB40 https://youtu.be/fl7uezsm3Gc best I can do for a screen shot is an early demo

shane-droid avatar Jan 12 '23 22:01 shane-droid

Made quick test with new UI and it will be pretty hard to fit 10 character label even using TINSIZE font without loosing data overview. LS_labels

.. and even more troublesome in tight-packed screens with UI elements where LS is a source on the list. Screenshot 2023-01-13 at 09 01 58

JimB40 avatar Jan 13 '23 08:01 JimB40

Is it possible to add a new line the grid? With an If statement, for example :

If LS_name ! = null or whitespaces { Grid. Newline() Print LS_name }

On Fri, 13 Jan 2023, 3:05 am Robert, @.***> wrote:

Made quick test with new UI and it will be pretty hard to fit 10 character label even using TINSIZE font without loosing data overview. [image: LS_labels] https://user-images.githubusercontent.com/46420768/212267871-10d426b8-2c4e-4f46-ac87-9f637f3ea77f.png

.. and even more troublesome thight packed screens with UI elements where LS is a source on the list. [image: Screenshot 2023-01-13 at 09 01 58] https://user-images.githubusercontent.com/46420768/212269042-665dfc02-ab5f-4a9d-909e-69dd613046f8.png

— Reply to this email directly, view it on GitHub https://github.com/EdgeTX/edgetx/pull/2085#issuecomment-1381450574, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOXAO7SXEM5MMZJYUDTJU7LWSEEDVANCNFSM5ZTYGA2Q . You are receiving this because you were mentioned.Message ID: @.***>

shane-droid avatar Jan 13 '23 12:01 shane-droid

@JimB40 also on inputs screen can the button width be adjusted to name size instead of switch size?

shane-droid avatar Jan 13 '23 12:01 shane-droid

@pfeerick hi Peter, can this pull request be merged across, thank you

shane-droid avatar Aug 03 '23 01:08 shane-droid