Skip to content

NV14: Deadzone option#1777

Merged
pfeerick merged 14 commits intomainfrom
nv14-deadzone
Apr 6, 2022
Merged

NV14: Deadzone option#1777
pfeerick merged 14 commits intomainfrom
nv14-deadzone

Conversation

@pfeerick
Copy link
Member

@pfeerick pfeerick commented Mar 31, 2022

Summary of changes:

  • adds deadzone option from open-flysky/Flysky-OpenTX-Test-Branch to radio firmware
  • teaches Companion about it enough to not nuke the radio setting

Currently, untested. Is now upgraded to status of "deadzone works, but monkey implementing settings storage screwed up". Weekend problem.

Also, radio hardware option would probably be better as a Choice rather than NumberEdit since it is limited to 7 options. However, it should be sufficient to see if this is the resolution.

May resolve #1756

@Ktr128
Copy link
Contributor

Ktr128 commented Apr 1, 2022

I compiled edgetx with deadzone.
Deadzone works great!!! )) Didn't notice any problems.

But there are problems with saving the settings.

On reboot, the settings are lost:

  • voice language spontaneously set to Czech;
  • switches SG to toggle;
  • switches SH to none.

@pfeerick
Copy link
Member Author

pfeerick commented Apr 1, 2022

Well, I can update "Currently, untested. " to "Tested, but settings borked". 🤪 Thanks for the feedback... it's probably in how I modified the yaml struct to save the setting... I was honestly surprised it let me just add add it without spitting the dummy... so have to play with that tomorrow or Sunday.

@Ktr128
Copy link
Contributor

Ktr128 commented Apr 1, 2022

I agree with you, you can add that it was tested and bugs were found while saving the settings.
Compiled with both nightly build and v2.7.0-rc1...
I tried to help you, look at the YAML struct, but I didn’t find anything wrong there.
But this is not surprising, because I'm a "great" programmer, only monkeys from the zoo are taller than me)))))))

@pfeerick pfeerick marked this pull request as draft April 1, 2022 10:07
@Ktr128
Copy link
Contributor

Ktr128 commented Apr 1, 2022

Found two suspicious things in radio/src/targets/nv14/CMakeLists.txt:

  • 3: option(STICKS_DEAD_ZONE "Enable sticks dead zone" YES)
    Perhaps it is necessary to write ON instead of YES?

  • three lines of code
    if(STICKS_DEAD_ZONE)
    add_definitions(-DSTICK_DEAD_ZONE)
    endif()
    repeated in the file 2 times.

I tried to change it, but the bug didn't go away...

@Ktr128
Copy link
Contributor

Ktr128 commented Apr 2, 2022

Is there a need to check #1777 on NV14 now?

@raphaelcoeffic
Copy link
Member

Is there a need to check #1777 on NV14 now?

It should work now with the latest changes. But I think we should avoid adding the extra byte if we can avoid it: there are some padding bits (implicit) at the end of the structure.

@pfeerick
Copy link
Member Author

pfeerick commented Apr 2, 2022

@Ktr128 I'm off to bed now, but I'm hoping the next build is looking more like final state if you want to give it a spin...

@Ktr128
Copy link
Contributor

Ktr128 commented Apr 2, 2022

pfeerick goodnight!
When the need arises to test, I will be happy to help you.

@pfeerick
Copy link
Member Author

pfeerick commented Apr 3, 2022

@raphaelcoeffic I think this raises a couple of points to discuss...

  • Was this the right way to settle down the tests?
  • Is setting a small deadzone (value 2, so 4) by default when deadzone feature enabled how we want it?
  • Placement of the option - I opted to place it directly under the stick naming options since it was stick related, but I am wondering if it should be moved down next to ADC filter... (and it's now a Choice BTW, rather than NumberEdit, since it's a limited list of options)?
    image

Another thought, if it stays where it is... full or half width?
image

@Ktr128
Copy link
Contributor

Ktr128 commented Apr 3, 2022

If the user's point of view is appropriate at this stage...
I quickly found the Deadzone location, despite expecting it to be in an old location that I knew about earlier. Therefore, from my point of view, you can leave the current one.
From the point of view of the beauty of the interface, I think half-width is better.
P.S. The main thing is that the dead zone is alive again! ))

@pfeerick
Copy link
Member Author

pfeerick commented Apr 3, 2022

If the user's point of view is appropriate at this stage...

IMO it's the most important view! 😄 You have to use it, after all... if the option being there was quickly found, sounds like it is in the right place. And I'm more inclined towards half-width also... seems consistent and it's not like the width is needed.

@pfeerick pfeerick marked this pull request as ready for review April 4, 2022 11:44
@Ktr128
Copy link
Contributor

Ktr128 commented Apr 4, 2022

Compiled the firmware with the latest release of the Dead Zone.
Everything works! 👍

20220404_201509

I suggest renaming the pull request "Live Zone"! 😄

@pfeerick pfeerick added this to the 2.7 milestone Apr 5, 2022
@elecpower
Copy link
Collaborator

elecpower commented Apr 5, 2022

@pfeerick would you like me to finish the Companion side (Hardware) for you?

@pfeerick
Copy link
Member Author

pfeerick commented Apr 5, 2022

Oops... Nearly forgot that... Yes please! But please hold off until I check in with Raphael on status of PR incase I need to change radio side...

@pfeerick
Copy link
Member Author

pfeerick commented Apr 5, 2022

@elecpower All good... go for it... it's staying as it is.

@pfeerick pfeerick added the enhancement ✨ New feature or request label Apr 5, 2022
@elecpower
Copy link
Collaborator

@elecpower All good... go for it... it's staying as it is.

Try this out and advise if needs tweaking

@pfeerick
Copy link
Member Author

pfeerick commented Apr 6, 2022

No tweaking necessary... reading, changing and writing new value works perfectly... thanks! :)

@pfeerick pfeerick merged commit d9b51f8 into main Apr 6, 2022
@pfeerick pfeerick deleted the nv14-deadzone branch April 6, 2022 00:30
pfeerick added a commit that referenced this pull request Apr 6, 2022
Imported from open-flysky/Flysky-OpenTX-Test-Branch

* companion: Don't nuke `stickDeadZone` setting

* Implement data storage properly

* Add missing check for NV14 storage struct sizes

* Formatting

* Use one less byte in the packed data struct
Thanks @raphaelcoeffic :)

* Move setting of stickDeadZone default

* Use a Choice combo rather than NumberEdit

* Good riddance magic number

* Also set default stick zone if setting is missing

* Teach tests about `STICK_DEAD_ZONE`

* Tweak choice width

* Cpn NV14 add stick dead zone to hardware and section labels

* Cpn NV14 align default pot names to radio

Co-authored-by: elecpower <neilh713@tpg.com.au>
@SlipstreamFPV
Copy link

Can this be added for all radios?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement ✨ New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NV14 Throttle and Rudder issue

6 participants