-
-
Notifications
You must be signed in to change notification settings - Fork 340
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
feat(cpn): support more than just FrSky S.Port telemetry simulation #5410
Conversation
33ce46f
to
8b912b4
Compare
Post merge of #5381 sim telemetry will need to be modified to suit surface radios eg flight vs drive and hide flight sensors eg vario |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UI comments:
- change buttons font size to default (just look out of place in Linux)
- QComboBox protocol selector would look better IMO if moved closer to label
They already are the default font size, the .ui file does not specify a font size for the buttons e.g.
This is the same font size that the buttons are in the current frsky-only telemetry screen (and the fields and labels are 8pt as they are on the current frsky-only telemetry screen). I'm happy to change them to 8 point so they match the rest of the fields on that widget it that's what you want. Please confirm.
Just right-align the labels so they're next to the QComboBox OK? |
Make buttons consistent with rest of dialog.
Right align would suffice. IMO the main dialog could do with some grids to aid in layout but that is for another day. |
whould it be possible to use serial port as telemetry source and possibly output port for protocols like CRSF? |
If you wanted to do that I think you'd probably be better off changing radio/src/targets/simu to use a real serial port as an internal (or external) module - then you'd have real telemetry and the simulation telemetry wouldn't be used at all. |
My only nitpicks so far would be:
Otherwise, after a quick poke around and replay of the log... this is working fantastic! :) |
Fixed
It's being read OK, but I had the UI set up wrong given CRSF RSS measurements are an actual measurement not a percentage-like value like S.Port. Fixed.
Oops, didn't mean to change that part of the original UI. Padding added. |
Hey, you said you wanted it merge-ready :P . Keep an eye out for followup PRs. I also want to change what's selectable in the internal/external module dropdowns based on what the simulated radio is capable of and maybe a warning if you choose something that the current model isn't configured for. |
Looks quite nice to me. Unsure if in the scope of this contest, but I think ELRS has this specificity that you can set frame rate and telemetry frame ratio. When set to very low frame rate (like 25hz) and low telemetry ratio, the flow of incoming telemetry is quite slow, and may potentially affect Lua scripts. It would be great if (even at a later time) you could have a way to specify 'I'm running at 25Hz, 1/32 telem' (or whatever those values are) But already quite nice ! |
That's probably out of scope, but would be interesting to see. Do you see that primarily only affecting the replay of a log? Thus effectively just more granularity over the replay rate? Or are you after it slowing down/controlling the refreshes of the sensors, i.e. for ELRS to be taking into account the LINK and DATA groupings and data rates for sensors. |
I was thinking 'live' usage, affecting the refresh of sensors, for LUA devs who might make computation based on sensor that might glitch at low or high refresh speed. But thinking about it, that may not be practical since you need to hand change the values. Unless is introduced (definitely out of scope) an variation feature where you can ask sensors to 'jitter' a certain quantity |
You could change the log controller to schedule changes in line with the timestamp field of the CSV (rather than assuming the rate based on the first 20 rows), and then generate a CSV with "jittered" timestamps and/or values.. I'm not sure how you'd be able to generate a log from an observed problematic sensor, as the loggin process is a periodic sampler rather than logging the telemetry with a timestamp as it comes in. |
Oops wrong button sorry |
I was not thinking log at all, only live data, that could have a "jitter" (by that i don't mean like a problematic sensor, but like any sensor which value changes over time) value of 0 (so like today) or say x% where value would vary by that much, as it is currently difficult when you debug scripts on simu to see how your values follow (or not) sensor value changes (but then again , out of scope of the competition itself) |
@3djc you mean kind of like how the simulated GPS can currently be set to run its own little simulation which updates the position over time? |
Yes, something very similar, it could look like that (quick concept) where you could decide an amount of variation. If you wanted to go over the top, you would add another column to decide variation type (rng, sine, triangle, square,..), but a default of triangle or rng would be perfect |
9db59a6
to
b0179ea
Compare
Rebased to squash all the fixups |
Probably my final comments on this before merge:
|
Whoops, had some initialisation for the FrSky provider called by the wrong spot: now it populates those instance id fields like it did before this PR.
Made the simulated GPS accept space separated lat/long as well as tolerating spaces around a comma. |
Seems to be working perfectly... thank you! :) |
The code to check for the old (pre-OpenTX 2.1.9, Sep 2016) format of GPS coordinates in log CSVs is no longer used since EdgeTX#5410 so remove it. Also, drive by spelling fix.
Fixes #4983
Fixes #2156
Summary of changes:
Example: Screencast from 10-08-24 01:17:55.webm