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

Cannot change to a different model 2.7.0 T-Pro ELRS #1898

Closed
1 task done
sk8board opened this issue Apr 25, 2022 · 20 comments · Fixed by #2515
Closed
1 task done

Cannot change to a different model 2.7.0 T-Pro ELRS #1898

sk8board opened this issue Apr 25, 2022 · 20 comments · Fixed by #2515
Labels
bug 🪲 Something isn't working
Milestone
2.8

Comments

@sk8board
Copy link

sk8board commented Apr 25, 2022 •
edited
Loading

Is there an existing issue for this problem?

  • I have searched the existing issues

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

After updating the T-Pro ELRS to EdgeTX 2.7.0, I cannot change to a different model. When I select a different model, there is a pop-up screen that states, "Loading model...", then the screen returns to the original model.

When creating a new model, I get the same result.

If I down-grade to the firmware that came with the T-Pro, the problem is not present.

I also have a T-Pro 4in1, which does not have this problem when upgrading to EdgeTX 2.7.0.

The problem is only present on the ELRS version of the T-Pro.

Expected Behavior

When selecting a new or different model, I expect to change to the model I selected.

Steps To Reproduce

  1. Upgrade a T-Pro ELRS version to EdgeTX 2.7.0
  2. Select a new or different model

Version

2.7.0

Transmitter

Jumper T-Pro

Anything else?

No response
@sk8board sk8board added bug 🪲 Something isn't working triage Bug report awaiting review / sorting labels Apr 25, 2022
@sk8board
Copy link
Author

@pfeerick this bug does not seem to have any attention. Does the team need anything more?

1 similar comment
@sk8board
Copy link
Author

sk8board commented Sep 5, 2022

@pfeerick this bug does not seem to have any attention. Does the team need anything more?

@pfeerick
Copy link
Member

pfeerick commented Sep 5, 2022

Have you tried updating to 2.7.1, or a nightly build to see if it has been resolved?

@sk8board
Copy link
Author

sk8board commented Sep 5, 2022

I tried the latest nightly build today and the problem is still present.

@raphaelcoeffic
Copy link
Member

I tried the latest nightly build today and the problem is still present.

Thx for trying. We will need your settings/models to be able to reproduce.

@pfeerick
Copy link
Member

pfeerick commented Sep 6, 2022 •
edited
Loading

In other words, can you zip up a copy of the MODELS and RADIO folders from the radio that's playing up and upload/attach it here, please ;) I can't reproduce this on my ELRS T-Pro... it correctly flashes up the Loading screen, and the asterisk moves from the last model to the one selected or loading. And when I back out of the select menu, the correct model is loaded. So perhaps a model file is corrupt or something specific in the config is making it fail.

@sk8board
Copy link
Author

sk8board commented Sep 6, 2022 •
edited
Loading

Archive.zip

@raphaelcoeffic @pfeerick I have attached the Models and Radio folders as a zip file as you requested.

1 similar comment
@sk8board
Copy link
Author

Archive.zip

@raphaelcoeffic @pfeerick I have attached the Models and Radio folders as a zip file as you requested.

@pfeerick
Copy link
Member

pfeerick commented Sep 26, 2022 •
edited
Loading

Thanks for the reminder 😮 🙏

Right, so I'm juggling a few things atm, and am still puzzled as to why the semver version field is zero instead of 2.7.0, 2.8.0 etc, but that should be a cosmetic issue. What is a problem is not that it won't change models... but after doing the first model switch (as I was able to switch models exactly one time with that file set on the radio) ... it would show the loading screen as you mention, freeze, and it would trigger an emergency mode reboot... as signified by a ! shown at the top right of the screen.

What seems to be the cuplrit is that the trainer mode for all of the models is set to Master/SBUS ... when I turned that to OFF, on a model by model basis, I was able to switch models. I.e. change it to off, switch model. Turn it off for that model, switch model. And it seems to be fine after that. If a create a new model from scratch, i can switch to and from it without issue. But if I turn Master/SBUS on for that model, same deal, freeze and EM. So for now if you at least make that change, you'll be right to go again. We probably need to check the hardware definitions as something is probably misconfigured re: SBUS on that radio, or it shouldn't be an option but isn't being sanity checked.

@sk8board
Copy link
Author

@pfeerick Thank you for checking into this.

I use Master/SBUS trainer mode all the time because I only fly with my son who is still training. So, I will have to stick with the original firmware that came with my T-Pro ELRS.

Please let me know when a fix is available so I can update to newer firmware.

@pfeerick
Copy link
Member

pfeerick commented Sep 27, 2022 •
edited
Loading

Hm... so how do you use SBUS with the T-Pro? 👀

@sk8board
Copy link
Author

sk8board commented Sep 27, 2022 •
edited
Loading

@pfeerick @raphaelcoeffic

Hm... so how do you use SBUS with the T-Pro? 👀

I want to be clear, I have verified Master/SBUS trainer mode already works on T-Pro using the original firmware 2.6.1 (Labeled 2.7.0) that came with the T-Pro ELRS. You have verified there is a bug that prevents changing models when Master/SBUS trainer mode is enabled using newer firmware than original.

I was able to change my receiver (which is connected to the JR module connector) to PPM and switch the T-Pro trainer mode to Master/CPPM as a workaround for this problem. I have verified this workaround works very well.

I know the ELRS team will not support SBUS or PPM for receivers, which is why I created this enhancement request for trainer mode. ##1607

If enhancement request ##1607 CANNOT be supported, then it is important for the T-Pro and other radios to support Master/SBUS trainer mode. This is because no one makes an ELRS to PPM converter. Matek makes an ELRS to SBUS converter which can be used with an ELRS receiver and Master/SBUS trainer mode for wireless trainer function.

Since the original firmware that came with the T-Pro ELRS worked with Master/SBUS trainer mode, it seems the developement work of SBUS with the T-Pro has already been completed and just needs to be reused in future releases.

If enhancement request ##1607 CAN be supported and the EDGE team does not want to support SBUS on the T-Pro, then please replace Master/SBUS with Master/CRSF in the menu so people know to use CPPM or CRSF rather than chase a problem related to SBUS that will not get resolved.

I appreciate your help!

@sk8board
Copy link
Author

@pfeerick @raphaelcoeffic

I made a video to better explain my writing above.

Here is the link to the video

@pfeerick
Copy link
Member

Yes, that video clearly explains what you are after, thanks! :) I was more interested in the how it was connected, as I wasn't aware of any documentation on SBUS being a supported function on that transmitter. I noticed it doesn't crash if you re-enable Master/SBUS afterwards, so does it actually work then? i.e. It only crashes when changing models, but otherwise functions? Just means we are getting closer to identifying exactly where it is going wrong.

re: CRSF trainer input, did you give the PR that was linked to that issue a try to see if the works? #1462 It will need some re-work as it needs to fit in with the changes to the serial model, but I did have CRSF student input working on a TX16S via the module bay.

@sk8board
Copy link
Author

sk8board commented Oct 1, 2022

@pfeerick

I tried to use trainer mode Master/SBUS with 2.8.0-rc1. The result was erratic channel operation when trainer mode was enabled.

When using the original T-Pro ELRS firmware, Master/SBUS trainer mode worked as expected. Maybe the SBUS code from the original T-Pro ELRS firmware can be used?

re: CRSF trainer input, did you give the PR that was linked to that issue a try to see if the works? #1462 It will need some re-work as it needs to fit in with the changes to the serial model, but I did have CRSF student input working on a TX16S via the module bay.

With regard to testing a PR, I assume you mean to compile the PR for use? If so, I am not savvy enough to compile.

@pfeerick
Copy link
Member

pfeerick commented Oct 1, 2022

AFAIK, nothing was changed in the SBUS code. Note likely it is some clash/incompatability with the lower level serial driver rewrite and the tpro configuration - which wouldn't have been picked as SBUS isn't exactly plug and play on that radio ;) I'm on phone atm, but will link the guide for self-compile... It's pretty easy now :)

@pfeerick pfeerick removed the triage Bug report awaiting review / sorting label Oct 11, 2022
raphaelcoeffic added a commit that referenced this issue Oct 11, 2022
@raphaelcoeffic
fix: prevent generic module de-init if protocol is NONE
d497534 
Fixes #1898

De-initialising the external module while SBUS trainer is used on external module RX leads to the following sequence:
- `extmoduleStop()` is called, thus shutting down the timer DMA
- on certain targets, the timer DMA stream is the same as the external module RX DMA stream
- when this DMA stream is used by the SBUS trainer input, this leads to DMAFifo to return random bytes continuously
- thus driving `processSbusInput()` into an endless loop, which is run in the mixer task
- as the mixer task has the highest priority, it runs forever
- after some time the watchdog timer expires, and causes a reboot
@raphaelcoeffic raphaelcoeffic mentioned this issue Oct 11, 2022
Merged
pfeerick pushed a commit that referenced this issue Oct 12, 2022
@raphaelcoeffic
fix: prevent endless loop with SBUS trainer on model change (#2515)
655965d 
* fix: prevent generic module de-init if protocol is NONE

Fixes #1898

De-initialising the external module while SBUS trainer is used on external module RX leads to the following sequence:
- `extmoduleStop()` is called, thus shutting down the timer DMA
- on certain targets, the timer DMA stream is the same as the external module RX DMA stream
- when this DMA stream is used by the SBUS trainer input, this leads to DMAFifo to return random bytes continuously
- thus driving `processSbusInput()` into an endless loop, which is run in the mixer task
- as the mixer task has the highest priority, it runs forever
- after some time the watchdog timer expires, and causes a reboot

* DMAFifo: check if the stream is enabled

This provides some hardening to avoid the conditions described in previous commit.
@pfeerick
Copy link
Member

pfeerick commented Oct 12, 2022 •
edited
Loading

@sk8board Would you mind checking everything is working ok now? You can either use the next nightly, or 2.8.0-RC3 which is likely to come out later today.

pfeerick pushed a commit that referenced this issue Oct 12, 2022
@raphaelcoeffic @pfeerick
fix: prevent endless loop with SBUS trainer on model change (#2515)
0371814 
* fix: prevent generic module de-init if protocol is NONE

Fixes #1898

De-initialising the external module while SBUS trainer is used on external module RX leads to the following sequence:
- `extmoduleStop()` is called, thus shutting down the timer DMA
- on certain targets, the timer DMA stream is the same as the external module RX DMA stream
- when this DMA stream is used by the SBUS trainer input, this leads to DMAFifo to return random bytes continuously
- thus driving `processSbusInput()` into an endless loop, which is run in the mixer task
- as the mixer task has the highest priority, it runs forever
- after some time the watchdog timer expires, and causes a reboot

* DMAFifo: check if the stream is enabled

This provides some hardening to avoid the conditions described in previous commit.
@sk8board
Copy link
Author

@sk8board Would you mind checking everything is working ok now? You can either use the next nightly, or 2.8.0-RC3 which is likely to come out later today.

@pfeerick Yes, I will test 2.8.0-rc3.

@pfeerick pfeerick added this to the 2.8 milestone Oct 13, 2022
@sk8board
Copy link
Author

@pfeerick I have tested 2.8.0-rc3 and verified this problem is fixed.

Thank you to all who solved this problem.

@pfeerick
Copy link
Member

pfeerick commented Oct 14, 2022 •
edited
Loading

Fantastic! Thanks for checking it so quickly :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug 🪲 Something isn't working
Projects
None yet
Milestone
2.8
Development

Successfully merging a pull request may close this issue.

fix: prevent endless loop with SBUS trainer on model change EdgeTX/edgetx
3 participants
@raphaelcoeffic @pfeerick @sk8board