edgetx
edgetx copied to clipboard
Mavlink support
This PR continues the MAVLink integration of #122.
TODOs
Cosmetics / Code structure
- [x] Remove Olli's markers (
// OW
/// OWEND
). - [x] Use the standard GPLv2 boiler plate instead of Olli's copyright notice.
- [x] Remove useless
USB_MAVLINK_MODE
and USB serial description specific to MAVlink (USB serial is just "USB serial"). - [x] Remove MAVLink includes from
radio/src/opentx.h
and add to appropriate files that need it. - [x] Move unrelated function in
radio/src/lua/api_mavlink.cpp
to somewhere else (does anyone really need this?). - [x] Clean-up changes
datastructs.h
and remove useless definitions.
UI
- [ ] Move MAVLink telemetry setup items from "Model Setup" to "Model / Telemetry".
Fix breaking changes
- [ ] Allow for using
CLI
and orDEBUG
, even if MAVLink support is enabled. - [ ] Fix
INTERNAL_GPS
when compiled with/withoutTELEMETRY_MAVLINK
. - [x] Check why
g_eeGeneral.mavlinkExternal == 1
should disable external module functionality (probably not needed). - [x] Fix / remove changes to
radio/src/targets/horus/hal.h
(see#if (defined(RADIO_T16) || defined(RADIO_T18)) && defined(AUX_SERIAL)
).
Code re-use
- [ ] Use existing Serial Rx instead of
mavlinkTelemUsbRxFifo
& inspect other added FIFOs (checkauxSerialTxFifo
as well). - [x] Check why
uint16_t telemetryStreaming
(normallyuint8_t
).
After @olliw42 decided not to finish his PR
wau ...
For the record, I was closing the other PR because you were asking for several very serious changes to the code which I don't see can easily be accommodated in that code without breaking or stripping of crucial functionality, and it was you and not me who has set the conditions and didn't find the suggested solutions acceptable. And now you are just happy to go with the code ignoring your own comments .... and in addition promote misrepresented intent.
sad to see this behavior
:-1:
For the record, I was closing the other PR because you were asking for several very serious changes to the code which I don't see can easily be accommodated in that code without breaking or stripping of crucial functionality, and it was you and not me who has set the conditions and didn't find the suggested solutions acceptable. And now you are just happy to go with the code ignoring your own comments .... and in addition promote misrepresented intent.
A long text to explain that you won't finish the job, which is basically the bottom line.
And now you are just happy to go with the code ignoring your own comments
You missed the point: this PR is not finished, take a look at the TODO-list.
Dear @olliw42, I was surprised and at the same time devastated to learn that you closed PR #122, so close to being merged. You wrote that a revised attempt should go into new PR, well let's take a chance to finish MAVLink integration to ETX master here! OTX/ETX community has long waited for native MAVLink in ETX/OTX (many attempts in OTX GH... also from many years ago already). As @raphaelcoeffic has now himself re-created this PR, this is a clear indication that MAVLink support is much desired in EdgeTX ;)))
Reading the comments at the end of #122, they do make a lot of sense to me and in short list two major issues to be tackled:
- use existing USB CDC/VCP Serial, instead of custom serial for MAVLink
- allow CLI & DEBUG to be used at the same time with MAVLink (on serial ports not used by MAVLink)
Olli, you have done absolutely marvelous work with MAVLink integration, this is your baby! It would be awesome if you could consider joining with ideas allowing the last remaining points to be solved.
The option to use JR-Bay for MAVLink is possible only with a custom hw, pictured here: https://www.rcgroups.com/forums/showpost.php?p=46948695&postcount=711 ~~This might be something not initially usable for users, as the schematic mod has not been published by @olliw42 yet. Thus it might be an idea first to comment out this functionality, as this would possibly also fix the JR-Bay power issue listed here:~~ https://github.com/EdgeTX/edgetx/pull/36#issuecomment-842437807
Update: The schematic and src. of the required STM32F1 µC are now available at Olli's GH page: https://github.com/olliw42/otxtelemetry/tree/master/SPortBridge
Use the standard GPLv2 boiler plate instead of Olli's copyright notice.
just for the record, changing copyright without permission by the copyright holders would constitute copyright infringment
Use the standard GPLv2 boiler plate instead of Olli's copyright notice.
just for the record, changing copyright without permission by the copyright holders would constitute copyright infringment
You are still the copyright holder, that will not change. However, the license is still GPLv2.
Tested 7a19a3ae on RM TX16S. AUX1 - MAVLink (ArduPilot via DragonLink), AUX2 - GPS, top USB - Mission Planner (MAVLink via virtual serial port). MAVLink data flows as expected, internal GPS functions.
Instead of using "CDC" (official USB class name for virtual serial port, instead of "Debug"), naming of USB connection mode in the selection dialog might be simplified:
- USB Joystick (HID) -> USB Joystick
- USB Storage (SD) -> USB Storage
- USB Serial (Debug) -> USB Serial
I gave further thought to my last post above - if DEBUG and/or CLI are built in, then we do need to distinguish which one the user wants to activate for USB, if he selects serial: Debug/CLI or MAVLink. Hmm... any ideas where to do it? For Serial port, there is a selection under SYSTEM ->Hardware -> Serial port, but not for USB (yet).
Instead of using "CDC" (official USB class name for virtual serial port, instead of "Debug"), naming of USB connection mode in the selection dialog might be simplified
Done!
I gave further thought to my last post above - if DEBUG and/or CLI are built in, then we do need to distinguish which one the user wants to activate for USB, if he selects serial: Debug/CLI or MAVLink.
We should have a setting similar to what we have for the normal serial ports.
Use the standard GPLv2 boiler plate instead of Olli's copyright notice.
just for the record, changing copyright without permission by the copyright holders would constitute copyright infringment
You are still the copyright holder, that will not change. However, the license is still GPLv2.
I repeat, changing copyright without permission by the copyright holders would constitute copyright infringement like it or don't, but that's the rules this is not my PR, so the rules apply that's the facts
Use the standard GPLv2 boiler plate instead of Olli's copyright notice.
just for the record, changing copyright without permission by the copyright holders would constitute copyright infringment
You are still the copyright holder, that will not change. However, the license is still GPLv2.
I repeat, changing copyright without permission by the copyright holders would constitute copyright infringement like it or don't, but that's the rules this is not my PR, so the rules apply that's the facts
I don't understand: are you trying to publish something in a GPLv2 software without licensing that change with GPLv2? This does not work. If you cannot accept that, we will have to reject the code, as it is pre definition licensed under GPLv2 when it is added to the same software.
Or is the fact that I added "Copyright EdgeTX" that is your issue? Please explain further.
@olliw42 By looking at diff https://github.com/EdgeTX/edgetx/pull/150/commits/f7c0036d89725a6d6422066732360402b6e56246 your copyright lines are still all there, only std. GPL v2 boiler, as OTX/ETX std., was added by @raphaelcoeffic. Can you please point to where do you see an issue with this or how would you prefer the header to further list your main authorship?
@olliw42 you might find this interesting: https://opensource.com/article/20/10/copyright-notices-open-source-software
"While these notices may appear important, their presence in source code today is largely a residue of the copyright law of the past. There was a time when failure to include a copyright notice in published material could result in complete loss of rights under US copyright law; that changed when the United States finally joined the many other countries that were already parties to the Berne Convention (US accession to the treaty came on November 16, 1988, and became effective in the US on March 1, 1989)."
@olliw42 if that helps, I can assure you I will merge this with a proper merge commit, so that your contribution will stay visible, commit by commit.
I repeat, changing copyright without permission by the copyright holders would constitute copyright infringement like it or don't, but that's the rules this is not my PR, so the rules apply that's the facts
"Changing copyright without permission by the copyright holders" is indeed copyright infringement. However, that has not happened here. You contributed code to a GPLv2 licensed project, hereby granting copyright to that project, and all derivative works. Your copyright to the code has not been modified, nor has the text indicating that been removed. The text indicating copyright has simply been reformatted to be consistent with rest of the source code for the project. Those are the facts, as documented by the commit logs.
Tested 04a91fb4 on RM TX16S, built with:
-DPCB=X10 -DPCBREV=TX16S -DDEFAULT_MODE=2 -DGVARS=YES -DPPM_UNIT=US -DHELI=NO -DLUA=YES -DINTERNAL_GPS=YES -DTELEMETRY_MAVLINK=YES -DCMAKE_BUILD_TYPE=Debug
AUX1 - MAVLink (ArduPilot via DragonLink), AUX2 - NMEA GPS, top USB-C - PC with Mission Planner.
By plugging in the top USB-C, now a popup with the following options comes up:
- USB Joystick
- USB Storage
- USB Serial
MAVLink (still) works via AUX1, also via USB, internal GPS works. :)
Text in full screen (modified) Yaapu LUA script is a bit shifted. This is also visible in @yaapu 's PR #151 screenshot (all text basically, but see e.g. the HUD top center heading "0", or the green digits in HUD, where they clearly are not there where they should be):
@yaapu mentioned a 3 pixel offset in Discord: https://discord.com/channels/839849772864503828/842693696629899274/850753828382965811
Cannot build yet with added -DDEBUG=YES -DCLI=YES
(expected, as the code still needs mods to allow this)
come on, guys, pl resort back to common sense
However, that has not happened here. You contributed code to a GPLv2 licensed project
no, I don't. This is @raphaelcoeffic 's PR, not mine
hereby granting copyright to that project
ergo I don't
all you are granted therefore are the rules of os
Your copyright to the code has not been modified
it has been
so, in your opinion, you buy a CD of music of someone who has the (c), you stick your (c) in addition to it claiming (c) and argue that's ok since the other (c) was not modified ... so effectively anyone can claim (c) of any others works since all one needs to do is to stick one's own (c) onto it in addition to the previous (c) ... I assume that all can agree that this is obviously nonsense, I at least think this can't be so difficult to understand
since there seems to be confusion about what I'm talking about, I explictly state that I was not commenting on attribution, I am commenting on copyright
manoman
@olliw42 Can you please propose a solution, how this could be fixed to your liking, together with the std. EdgeTX boiler?
@olliw42 Can you please propose a solution, how this could be fixed to your liking, together with the std. EdgeTX boiler?
that's impossible, that's the nature of it you can't have it together with the std ETX boiler since it has (c)
@olliw42 Would you please propose an alternative boiler plate that is to your liking? I'm confident, we are able to work smth. out so that everyone will be happy at the end!
Would you please propose an alternative boiler plate that is to your liking?
I could but I won't since this part is 100% up to your liking and not mine
I'm confident, we are able to work smth. out so that everyone will be happy at the end!
it's easy to achieve and it is surprising that it (again) is such a difficult process. Create anything you want as long as it does not change copyright, as easy as that.
We need something to discuss and/or agree on, so allow me to make a proposal, tackling the issue with double (c) lines you mentioned:
* (c) www.olliw.eu, OlliW, OlliW42
*
* Part of EdgeTX project - https://github.com/EdgeTX/edgetx
* Based on code named
* opentx - http://github.com/opentx/opentx
* th9x - http://code.google.com/p/th9x
* er9x - http://code.google.com/p/er9x
* gruvin9x - http://code.google.com/p/gruvin9x
*
* License GPLv2: http://www.gnu.org/licenses/gpl-2.0.html
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
Would that be OK with you?
it is not part of the EdgeTx project, it is part of the MAVLink for OpenTx project, and it has been taken over from there
what I find most projects to do in such cases is to keep the original header and add below something commenting (only) on additions/modifications to the code, or something like adapted to blabla or whatever, I don't have a specific example in mind but a bit google should give ideas
How about:
* (c) www.olliw.eu, OlliW, OlliW42
*
* Based on EdgeTX pull request #122
*
* EdgeTX project - https://github.com/EdgeTX/edgetx
* Based on code named
* opentx - http://github.com/opentx/opentx
* th9x - http://code.google.com/p/th9x
* er9x - http://code.google.com/p/er9x
* gruvin9x - http://code.google.com/p/gruvin9x
*
* License GPLv2: http://www.gnu.org/licenses/gpl-2.0.html
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
Check why g_eeGeneral.mavlinkExternal == 1 should disable external module functionality (probably not needed).
Isnt that needed for the uart via s.port pin in external module bay? As from Olli planned (He also developed a Platine Layout for This) To make a serial Radio hf module possible? Then this maybe Collides with tbs and r9 modules. Again: maybe utilize the first and second hf bay pin in combination with express and externalaccessmod Radios would be better? EG: Set External HF Type - > Serial Radio Module Or Set External HF Type - > Crossfire w. MAV over Serial (rc and crsf telemetry still over s. Port)
This would also solve the question for deactivating external modules. (Then its deactivated only if a specific Module isnt selected)
If course second external inversion would be needed for a Bay Serial Radio Module.
@brainbubblersbest the plan was to include it once the hardware is available, which could give us some time to make better / more flexible serial port drivers.
The Hardware for two wire Serial is already available. 1- All we need here is: -a Serial Radio module -two tbs inverters -a express Radio or a Radio with the externalaccessmod. (easy wiring to do on non frsky Radios) -an old HF Module Case to mount everything inside.
2- Or a r9m 2019 hf module to modify and make use of the already included Inverters here.
3- In Addition: i ordered an hf Module Adapter for the Fullsize crossfire to figure out if the mav-link data here can easy linked out to pin 1 and 2. Just for tinkering a bit how complicated this would be.
from which you can learn that EdgeTx is not leading edge as it claims to be
they don't deliver to the users fully functioning features just because of developer affectedness like that a debug functionality is not to their liking or serial port drivers are not as nice as they wished them to be
I fully agree that some solutions of mine are not the best code wise, and I would have loved if I would have been able to do better, but users give a dam about this, they want the features, working features, and the solutions available today do already one thing: they work and deliver.
In remarkable contrast, in many other cases, code solutions which are just "not so nice" from any software engineering point of view are happily accepted to the code. Not nice, but they do the job, for now, so accepted. Not so here.
Concerning the external bay feature, you know that I was not so enthusiastic when you first suggested it, but I reverted my opinion, and publicly confessed so. It is a most important feature ... and from the very first second on I loved it and would not want to miss anymore ... this is simply the future ... and will open all sorts of super convenient solutions for users, many possible DIY. But not with EdgeTx. If you want leading edge you need to resort to my branch.
It's really sad. The MAVLink addition (and what it all came with) is likely by far the largest single novel addition, and the only novel addition of that magnitude so far, and likely also by release time ... and already in this very first biggest example where a leading edge attitude could have been demonstrated the EdgeTx project miserably fails ... because of developer affectedness and stubborness.
you are correct, the hardware is DIY no issue another excellent hardware base you've not mentioned is the siyi fm30