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.

Sign up for GitHub

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

Jump to bottom

fix(el18): switch ADC channel and direction #3779

Merged
merged 1 commit into from
Jul 25, 2023
Merged

fix(el18): switch ADC channel and direction #3779

merged 1 commit into from
Jul 25, 2023

Conversation

raphaelcoeffic
Copy link
Member

No description provided.

pfeerick
pfeerick approved these changes Jul 25, 2023
View reviewed changes
Copy link
Member

@pfeerick pfeerick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

B & D switches are where they should be on EL18 (and also NV14), with all switches going the right way now.

@pfeerick pfeerick added this to the 2.10 milestone Jul 25, 2023
@pfeerick pfeerick added the bug/regression ↩️ A new version of EdgeTX broke something label Jul 25, 2023
@pfeerick pfeerick merged commit 51b4341 into main Jul 25, 2023
@pfeerick pfeerick deleted the fix-el18-switches branch July 25, 2023 06:07
@philmoz
Copy link
Collaborator

philmoz commented Jul 26, 2023

This PR causes SWB & SWD to be swapped on the EL18 I received today.

If I revert this PR then the two switches behave correctly.

@raphaelcoeffic
Copy link
Member Author

This PR causes SWB & SWD to be swapped on the EL18 I received today.

I think we really need to clarify what is the "normal" way...

If I revert this PR then the two switches behave correctly.

This PR has 2 parts:

  • exchanging SWB & SWD,
  • and correcting ADC_DIRECTION (TX_VOLTAGE and TX_VBAT must be placed before the analog switches).

So once we clarified what is the "production" switch types & order, we'd need to recheck.

@philmoz
Copy link
Collaborator

philmoz commented Jul 26, 2023

With this PR, on my EL18 then the physical switch labelled SWB controls the SD switch in the firmware (Hardware, Inputs page). And physical switch SWD controls SB in the radio.

SB is on the front of the radio top left, SD is front bottom right.

If I revert the ADC_GPIO_PIN_SWx and ADC_CHANNEL_SWx changes then SB and SD work as labelled.

Directions are all ok - I did not revert the ADC_DIRECTION change.

@raphaelcoeffic
Copy link
Member Author

revert the ADC_GPIO_PIN_SWx and ADC_CHANNEL_SWx changes then SB and SD work as labelled.

And the default switch types are OK as well? @pfeerick we really need some feedback from FlySky....

@philmoz
Copy link
Collaborator

philmoz commented Jul 27, 2023

The defaults in 2.10 are wrong for my EL18 if starting from scratch (new SD card).

  • S1 & S2 are set to None, instead of POT
  • SC is set to Toggle, not 3POS
  • SE is set to Toggle, not 2POS

Companion is the same with the additional issue that SC cannot be changed to 3POS. It can be changed to 3POS in the firmware; but not in companion (even if loading radio settings from the actual radio).

@philmoz
Copy link
Collaborator

philmoz commented Jul 27, 2023

Also the 2.10 firmware build on my EL18 lists T5 & T6 as trims.
Shows up in flight modes switch/source selection and the hardware debug keys page.

@raphaelcoeffic
Copy link
Member Author

@philmoz we need to finish / integrate #3783, as this introduces the necessary bits to differentiate EL18 from NV14 at build time, and thus implement the different default values and possibly the different switches (SWB vs SWD). I believe the current state in main serves the NV14 correctly (my EL18 has the same switch config as the NV14).

@raphaelcoeffic
Copy link
Member Author

Also the 2.10 firmware build on my EL18 lists T5 & T6 as trims. Shows up in flight modes switch/source selection and the hardware debug keys page.

This probably need to be addressed separately, as this is no different from the NV14.

@philmoz
Copy link
Collaborator

philmoz commented Jul 27, 2023

@philmoz we need to finish / integrate #3783, as this introduces the necessary bits to differentiate EL18 from NV14 at build time, and thus implement the different default values and possibly the different switches (SWB vs SWD). I believe the current state in main serves the NV14 correctly (my EL18 has the same switch config as the NV14).

Does that mean there are different EL18 switch configurations in production radios?
That will make things messy I would think.

@raphaelcoeffic
Copy link
Member Author

Does that mean there are different EL18 switch configurations in production radios?

So it seems. Mine is a pre-prod, so there is that.

@pfeerick
Copy link
Member

pfeerick commented Jul 27, 2023 •
edited
Loading

Back at #2675, the EL18 was changed from the switch configuration of the radios shipped to us from

SWC toggle => 3 pos
SWE is toggle => 2 pos

and we weren't informed of any other changes.

This PR is correct for the NV14 and 2x EL18 that I have - one an early development sample, the other was supposed to be a production line sample (which Raphael should have one of as well), but then they went and changed those two switches. So the question then is - are ours wrong, or is yours? This isn't the first time that I've received a radio with the switches upside down, so we really need input from both Flysky AND more users.

https://youtu.be/Qd8m7FVW3DQ

@philmoz
Copy link
Collaborator

philmoz commented Jul 27, 2023

The manual I downloaded from the FlySky web site shows SC and SE as 2POS toggle switches; but it is dated 15th Nov 2022.

@raphaelcoeffic
Copy link
Member Author

my EL18 lists T5 & T6 as trims. Shows up in flight modes switch/source selection and the hardware debug keys page.

Created #3860 to fix this.

@pfeerick
Copy link
Member

I've thrown the question to Flysky to answer, so hopefully we'll get some clarification on that soon.

@philmoz
Copy link
Collaborator

philmoz commented Jul 28, 2023

Now I'm really confused.

After adding #3783 (which includes #3807) to my local build and rebuilding the EL18 with PCBREV=EL18 then switches SWB and SWD were swapped again.

I had to re-instate the changes in this PR to get them back to normal.

@philmoz
Copy link
Collaborator

philmoz commented Jul 28, 2023 •
edited
Loading

And now I'm really really confused :(

When I powered the radio off and back on again SWB and SWD were swapped.

Then when I went to the SYSTEM - HARDWARE page they got fixed back to their correct assignments?????

This is not 100% reproducible - sometimes the switches are correct on power up.

It also appears to be a time based thing that fixes them - if they are wrong on startup, waiting a seconds will fix it.

@Delorean14
Copy link

Delorean14 commented Oct 25, 2023 •
edited
Loading

Sorry for the question, but does all of this mean that we can select the NV14 if we have an EL18? I am just getting into the hobby again and got an EL18 last week. I'm loving it so far, but I'm confused by this thread.

@pfeerick
Copy link
Member

So why are you here reading this thread? Sucker for punishment? 🤣

But seriously, while around 2.7 and maybe even 2.8 you probably could have, I think now the EL18 firmware build is actually different to the NV14 firmware build. Even so, there is a Flysky EL18 firmware target in EdgeTX buddy, as well as two different firmware files since at least 2.9.x.

image

@Delorean14
Copy link

So why are you here reading this thread? Sucker for punishment? 🤣

Hyuuuuuuuup. ;)

But seriously, while around 2.7 and maybe even 2.8 you probably could have, I think now the EL18 firmware build is actually different to the NV14 firmware build. Even so, there is a Flysky EL18 firmware target in EdgeTX buddy, as well as two different firmware files since at least 2.9.x.

I guess I was under the wrong impression that the companion could make setting up models a little easier at home in using the computer? So absolutely everything model related has to be done on the radio's touch screen? This is all making me feel extremely ignorant, so I apologize for it.

@pfeerick
Copy link
Member

pfeerick commented Oct 25, 2023 via email •
edited
Loading

@Delorean14
Copy link

Delorean14 commented Oct 25, 2023 via email

pfeerick added a commit that referenced this pull request Mar 8, 2024
@pfeerick
revert: partial revert of #3779
f4fe4c0 
This partially reverts commit 51b4341, to test if this is needed post #4563, as it seems there may have been another issue causing the switch swap.
@pfeerick pfeerick mentioned this pull request Mar 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pfeerick pfeerick pfeerick approved these changes

@richardclli richardclli Awaiting requested review from richardclli

Assignees
No one assigned
Labels
bug/regression ↩️ A new version of EdgeTX broke something
Projects
None yet
Milestone
2.10
Development

Successfully merging this pull request may close these issues.
4 participants
@raphaelcoeffic @philmoz @pfeerick @Delorean14