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.

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

[radio control] spektrum also usable for intermcu #847

Merged
merged 1 commit into from
Oct 26, 2014
Merged

Conversation

flixr
Copy link
Member

@flixr flixr commented Oct 7, 2014

Alternative to #843

You have to add the radio_control subsystem to the ap target as well in order to get the RADIO_CONTROL_TYPE_H define to load the correct implementation header so we get the RADIO_CONTROL_NB_CHANNEL.
For spektrum you might have to define RADIO_CONTROL_NB_CHANNEL in your airframe file to something less than 10 to use it with the uart inter_mcu.

@flixr
Copy link
Member Author

flixr commented Oct 7, 2014

@dewagter could you test this?

@OpenUAS
Copy link
Contributor

OpenUAS commented Oct 7, 2014

CDW still "out of office" ;), I'm in, but swamped with tasks. Next week more validation possible.

@flixr flixr added the Airborne label Oct 9, 2014
@flixr
Copy link
Member Author

flixr commented Oct 9, 2014

@gautierhattenberger how does this look to you?

@gautierhattenberger
Copy link
Member

It looks okay, but I still don't know (or forgot) why we don't use a generic spektrum radio file, just to generate the list of channel at the same place than other systems ? Will it help to do that in this case ?

@flixr
Copy link
Member Author

flixr commented Oct 9, 2014

Well, at least in my opinion you shouln't need to specify a radio xml file for spektrum at all...
The only place where this was used was for the number of channels for inter_mcu (with the number of channels coming from the spektrum implementation header undef-ed).
So rather than requiring a spektrum radio xml file that just duplicates the information and is only used in the dual mcu case, I think this is nicer and doesn't include the generated radio.h anywhere...
Now we still don't have a nice solution to not select a radio xml file at all, as it is still "required" by the build system, see also #78

@gautierhattenberger
Copy link
Member

Well... funny thing is that actually the radio xml is not mandatory to build an aircraft (nor flight plan, but others are). You can try:

  • remove by hand a radio attribute (not only the value, the complete attribute)
  • compile from a terminal (without opening the paparazzicenter)
  • radio.h is not generated ! and no complain from the build system

The "problem" is that if you open the aircraft in paparazzicenter, the radio attribute will be added again with the default value, and their is no way unselect it from there. An empty value for the attribute will also fail currently, and you can't really do that from the GUI anyway.
I looked some years ago to add a check box to activate or not the generation for a file, but the mixing of makefile/generator made this a bit painful. I should probably have another look...

@flixr
Copy link
Member Author

flixr commented Oct 10, 2014

Maybe we can treat and empty radio attribute the same way as if there is none?
Eventually it would of course be nice if you only get radio and flight plan selectors in the paparazzi center if you actually need them...

@dewagter
Copy link
Member

radio.h is used in all radio_control files except spektrum.

is it a problem to have a spektrum.xml radio file instead of header file hardcoded values just as all other radios?

@lamestllama
Copy link
Member

Christopher,

The only reason why the spectrum values are hardcoded is because they cannot be changed in the hardware. The various models of transmitter will always map the function of a stick or switch to the same channel number even across various modes of transmitter (mode 1 , mode 2 and mode 3).

For instance, say you fly using a transmitter with the throttle on the left stick and I fly one with throttle on the right stick then as long as both of us are using spektrum transmitters the throttle stick on both transmitters will map to channel 1 in paparazzi and we can swap transmitters with a rebind without changing paparazzi configuration.

If spektrum where to diverge from this then there receivers would no longer work across their range.

The reason I hardwired the values was for simplicity in that it required zero configuration in order to work.

To answer to your question from a technical point is that it is not a problem to have an XML file.

Eric

On 24 Oct 2014, at 9:37 am, Christophe De Wagter notifications@github.com wrote:

radio.h is used in all radio_control files except spektrum.

is it a problem to have a spektrum.xml radio file instead of header file hardcoded values just as all other radios?


Reply to this email directly or view it on GitHub.

@flixr
Copy link
Member Author

flixr commented Oct 24, 2014

As Eric explained the radio xml files is neiter needed nor used or useful for spektrum.
And everything in the radio xml files is really meant for ppm input, since you specify the sync pulse, number of channels and their pulse length.

I see zero benefit in using the generated/radio.h file for spektrum and the only case when it was actually included in the code was the fixedwing intermcu.
And even then the only thing really used from that was the number of channels, a fix/hack so you can send less than all available channels from fbw to ap (see e.g. #700).

So IMHO if we now suddenly start using only some information from the radio.xml file for spektrum, it just makes things a lot worse:

  • for rotorcraft (no intermcu) it does not matter as it is not even included
  • for fixedwing in single mcu case it does not matter as it is not even included
  • for fixedwing in dual mcu case it is actually used
    • to get the number of channels to send between fbw and ap
    • in your proposal Intermcu spektrum #843 also to know the channel names in ap, so can seriously wrong if they don't match the spektrum defintions

Unfortunaltely the ocam sim still want's to read a radio.xml file if you want to simulate the RC there...
That is actually the reason for the current spektrum.xml radio file.

Now the actual problems/questions with radio control in dual mcu case are:

  • How do you specify how many (and possibly even which selected) channels you want to send from fbw to ap?
  • How do you get the channel assignments in ap target

In this pull request that is "solved" by adding the appropriate radio_control subsystem to the ap target as well (simply add it for the whole firmware instead of only fbw target).
For dual mcu ap, only the implementation header is included which has the number of channels and the channel names/assignments.
If you don't need all channels, you can define how many you want to use in your airframe file.

Now for the number of channels we could also introduce a separate option/define to configure how many you want to send to ap instead of defining RADIO_CONTROL_NB_CHANNEL.
All current solutions only make it possible to ignore channels at the end (no skipping of channels in between).

Or is there some advantage of using the radio.xml file for spektrum that I'm missing??

flixr added a commit that referenced this pull request Oct 26, 2014
[radio control] spektrum also usable for intermcu

You have to add the radio_control subsystem to the ap target as well in order to get the RADIO_CONTROL_TYPE_H define to load the correct implementation header so we get the RADIO_CONTROL_NB_CHANNEL and channel names.
For spektrum you might have to define RADIO_CONTROL_NB_CHANNEL in your airframe file to something less than 10 to use it with the uart inter_mcu.
@flixr flixr merged commit 2b76c64 into master Oct 26, 2014
@flixr flixr deleted the intermcu_rc branch October 26, 2014 16:42
@flixr flixr added this to the v5.4 milestone Nov 20, 2014
@dewagter dewagter mentioned this pull request Nov 20, 2014
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants