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

5.1+4.2 quality upgrade #198

Merged
merged 20 commits into from
Jan 21, 2021
Merged

5.1+4.2 quality upgrade #198

merged 20 commits into from
Jan 21, 2021

Conversation

WouterJD
Copy link
Owner

@WouterJD WouterJD commented Jan 9, 2021

Modifications as described under https://github.com/WouterJD/FortiusANT/wiki#version-51-test-ii-available-for-evaluation
ready for promotion to the master branch.

Comments appreciated.

@WouterJD
Copy link
Owner Author

WouterJD commented Jan 9, 2021

#184 confirmed solved see

@jujroy
Copy link

jujroy commented Jan 9, 2021

Following up on v 5.1 testing.
Hello Wouter and TriScott,
I installed v5.1 today and proceeded with a few tests.
GUI in the top left corner displayed : Fortius Antifier v 5.1 Test II (Version on Github=Fortius Antifier version 5.0)
I did NOT get the Start Pedalling prompt. (BTW, I love this reminder). Nonetheless, I completed a few run off tests to get approx. 7 sec.
The displayed power was much similar to V4.1.1 and V5.0 - thus too high.
I keep myself posted especially with issues 14, 173, 193, 198.
As I mentioned in my Intro, I am NOT very computer smart and cannot start the application with arguments. I simply double-clic the Fortius.exe file in the windowsexecutable folder. When I read the manual, I found out that Python could be something else than a snake . I skipped the Python installation step and went for the easy Windows way.
My compliments again for all this work. This is all Back Magic to me. I will be more than happy to conduct a few more EASY tests for you, if so desired. In the meantime, I am using v3.8. Cheers.

@WouterJD
Copy link
Owner Author

I simply double-click the Fortius.exe file in the windowsexecutable folder

Thats good. Please then:

Click new settings button (above the other buttons); perhaps click "Stop" before, if you have autostart activated.
Select Motorbrake, press OK.

If you want to keep these settings for future starts, also select ""Save settings in JSON file

image

@jujroy
Copy link

jujroy commented Jan 10, 2021

Following up on v 5.1 testing.
Good morning Wouter and team
I installed the revised version today and proceeded with a quick test.
GUI in the top left corner displayed : Fortius Antifier v 5.1 Test II (Version on Github=Fortius Antifier version 5.0). The new cog wheel icon was in the top left corner. I ticked the boxes as per your note.
MAGNIFICIENT !
The displayed power was much similar to V3.8 and I love it.
You must have worked all night to accomplish all this remarkable work in such a short timeframe.
NOT being very computer smart, I love ticking boxes and I am sure that it will be useful to other guys out there.
My compliments again for all this work, you made my life much easier.
I will be more than happy to conduct a few more easy tests for you. Following some skiing, I will Zwift later today to experiment more. I will keep using this version. Cheers.

@WouterJD
Copy link
Owner Author

must have worked all night

I will not reveal the truthh on this subject; more nice things to come - watch the wiki!

@jujroy
Copy link

jujroy commented Jan 10, 2021

Hello Wouter. Me again. Got some great skiing done this morning.
Bad news. Not sure if I am doing something stupid or not. I powered everything up using the latest version. I did NOT get the start pedalling prompt....hummmm... Clicked on the parameter icon, Motor Brake was selected. I hit START and got high power again :( Did a few reboots, even re-install the software twice. Never saw the Start Pedalling prompt and always high power output. Fortunately, V3.8,that I keep aside, is still working for me. Again, many thanks

@WouterJD
Copy link
Owner Author

Hi @jujroy

I will be more than happy to conduct a few more easy tests for you.

Yes you can. Please try to reproduce the issue as described and run with debugging on (settings, Create logfile: check all boxes).
Reinstalling is not the issue here; it is something with data (not) being received.
I tried to reproduce on my T1932 but here it works.

Attach the logfile in the box under the text on github wher you type.

@jujroy
Copy link

jujroy commented Jan 10, 2021

Hello @WouterJD,

I cannot find how to attach a file in here. Did a paste on the small file and also mailed you this one and a larger one. Let me know if OK

@jujroy
Copy link

jujroy commented Jan 11, 2021

@WouterJD Hi again,

I made some progress. I can now attach files in this window. (new files this time)
Second, I found out, by fluke, a sequence to make my system work consistently.
-I turn OFF the power switch on the Power Pack;
-I launch the FortiusAnt software and wait until the GUI gives me the Start Pedalling Prompt
-I then power up the Power Pack, start pedalling, go thru the calibration process and guess what...I get the proper power output.

I tried this sequence 5 times and always got the right power output.
Tomorrow I will Zwift with V5.1,

Hope this is good news to you.

Cheers

FortiusAnt.2021-01-10 20-33-56.log
FortiusAntGUI.2021-01-10 20-34-03.log

@WouterJD
Copy link
Owner Author

WouterJD commented Jan 11, 2021

Version 5.1 Test III is available with major update on Motor/Magnetic brake determination.
(for details see wiki)

Please test


I have tested all individual components and will perform some real Trainer Road / Rouvy tests this week; did not want to keep you waiting for the Motor brake determination.

@jujroy
Copy link

jujroy commented Jan 11, 2021

@WouterJD , @TacxBiker

just to let you know that I did not get a chance to perform a Zwift test yet with slopes and high speed. My 5 tests were to ckeck the power up sequence and low speed. It behaved well. Will be on skis tomorrow. Will do a Zwift test Wednesday and will advise. Cheers

@trygvelu
Copy link

Hi, I have installed the v5.1 test III version to a RaspberryPi4 with 7"LCD following instructions to test BLE connectivity. Small bug: I get an error when loading with GUI, that the 'heart.jpg' image file can not be found. It seems FortiusAntGUI was edited to load 'heart.jpg' instead of 'Heart.jpg', which is the correct file name.

@WouterJD
Copy link
Owner Author

WouterJD commented Jan 12, 2021

I renamed Heart into heart; the windows visual source code did not see that as a change :-(
resolved with some fiddling

@WouterJD
Copy link
Owner Author

See wiki, two corrections made.
@BikeBeppe64 thanks for feedback

@jujroy
Copy link

jujroy commented Jan 13, 2021

Following up on v 5.1 IV testing.
Hello Wouter and contributors
I installed the revised version last night and proceeded with a Zwift test (Yorkshire).
GUI in the top left corner displayed : Fortius Antifier v 5.1 Test IV (Version on Github=Fortius Antifier version 5.0). The new cassette /chainring displayed 50 / 19. Did NOT test this feature and kept default settings.
Power Pack was in the ON position and I double-clicked the Fortiusant.exe file. I selected the MOTOR BRAKE feature and ticked all boxes to generate the log and JSON file. Relaunched Fortius Ant, got the
G I V E A P E D A L K I C K T O S T A R T C A L I B R A T I O N prompt, went thru calibration process.
In Zwift, I ensured that the coupling was done and I selected the Yorkshire course
https://www.strava.com/activities/4611028040
I had my Garmin Edge 810 on along with my Ant+ Garmin HR strap. HR displayed on Edge, Fortius Ant GUI and Zwift. All HR readings matched as expected.
I started cruising on the flat around 30 km/h. Garmin, Zwift and GUI speed readings were all in the same ball park. Power readings were good.
Hill climbs and descents gave me a good feeling and the power felt about the same as V3.8.
Something I never mentioned before but that I also observed with V3.8 is that the slope percentage on GUI is approx. half of what I see in Zwift. i.e GUI =4% and Zwift=8% - GUI =2% and Zwift=4%.
As you explained in the manual, one cannot expect the speed to match between the Garmin Edge and Zwift especially when climbing and going downhill, nonetheless, in real life when climbing 8%, I always need to go to the small chainring. In Zwift 8%, I always manage to do it 50/25 or less.
As I mentioned in my intro, I don’t have a Power Meter anymore but recalling rides on the flat and comparing with teammates (same height and weight) I guess that my setup gives me approx.. 15-20% unfair advantage in power. For my next Zwift session, I will use 50 / 15 virtual combination to see if my power perception changes.
With this in mind, using a 13 year old non-smart Fortius trainer, I am more than happy with all your hard work to make a wish come true. CONGRATS!
I will be happy to conduct further testing if you wish me to do so.
Attached are a few files and pictures for your review.
Cheers.

FortiusAnt.2021-01-12 16-32-55.log

FortiusAntGUI.2021-01-12 16-33-06.log

PXL_20210112_205157600

PXL_20210112_210510137

@WouterJD
Copy link
Owner Author

Thanks for detailed test report.

Something I never mentioned before but that I also observed with V3.8 is that the slope percentage on GUI is approx. half of what I see in Zwift. i.e GUI =4% and Zwift=8% - GUI =2% and Zwift=4%.

This is normal. Zwift has a setting that reduces the slope being sent to the trainer. The default value is 50%. So when Zwift shows 8% it sends 4% to the trainer.
We have implemented a similar setting (-G) for Rouvy users.

I guess that my setup gives me approx.. 15-20% unfair advantage in power.

This may be primarily influenced by the calibration, which offsetts the resistance.
If you would be sure about this, try to use -p90 which adjusts the power-calculation (limited to 90...110%).
It would be worth-while further investigating this deviation; if you like please raise a separate issue for this.

The power-difference (if stable) is not disturbinig too much when using Trainer Road; since initially the FTP is measured (15-20% too high) and then the training requires 15-20% too much power and therefore the training matches you capabilities.

In Zwift it's another story: 15-20% gives you an unfair advantage over other zwifters when doing a race. Of course, when going for yourself it does not make a difference, assuming you adjust your training to your capabilities. For this reason -p is limitted to 90-110% to keep it in "adjustment boundary"; 110% would give you a 10% advantage.

Question: I see you have the headunit out-of-reach; do you not use it and if not why not?
The headunit can be used to change gears: up/down for the rear; cancel for front and enter to reset.

With this in mind, using a 13 year old non-smart Fortius trainer, I am more than happy with all your hard work to make a wish come true. CONGRATS!

Thanks.

@Sat2Freak
Copy link

Hi Wouter,

I have been testing the latest build (Test version IV). What I like is the use of the cancel button, to 'jump' at a lower requested power. But the whole idea of virtual shifting is not my cup of tea.

The possibility to choose between "Low" or "High" (cancel button) and keeping the use of up and down buttons unchanged would simply be much better.

The range in virtual shifting is much narrower compared to the former implementation.
The motorbrake of my Fortius trainer makes a lot of noise downhill. In the other versions, I could ask for a higher power - but in the current version the possibilities are too limited.

Thx,

Rudy.

@WouterJD
Copy link
Owner Author

Cancel and up/down can both be used. Cancel was added upon request.
What do you mean by range (yes it is changed) please define and i will suggest

@Sat2Freak
Copy link

I know they both can be used, and I use it as intended. But I can't go lower than 34, and no higher than 12.
In case of a difficult climb, I want to go at a lower power, and in case of a downhill, i want to add more power.

In the meantime, I stick with test version II. This is by far the best ever made.

@WouterJD
Copy link
Owner Author

I want to go at a lower power, and in case of a downhill, i want to add more power

I will check later this week and correct.
The intention is that you can do 5 steps "beyond" the cassette-size.
If I see it now it does not work correctly yet.

Thanks for the observation that version II is the best ever made. Next version will be better yet.
I will also document the new setup of front/rear switching; it also allows you to define your own cassette as advanced feature.

Current default is "-T 34-50* x 34-30-27-25-23-21-19*-17-15-13-11"
Max # of sprockets is 12, there is no check on the values; most logical is that the values are descending.

An option for you might be "-T 50* x 30-27-25-23-21-19*-17-15-13-11-09-07" or anything similar

@WouterJD
Copy link
Owner Author

WouterJD commented Jan 16, 2021

Although "cassette-value" not displayed correctly, you have a range of 5...50 now; 11...34 visualized and beyond only sprocket value displayed (incorrectly, sorry).

By the way, if you define "-T 34-50* x 34-30-27-25*-23-21-19-17-15-13-11" you shift the starting-point and can increment further.

@WouterJD
Copy link
Owner Author

In the meantime, I stick with test version II. This is by far the best ever made.

Thanks for testing and providing feedback!

@jujroy
Copy link

jujroy commented Jan 16, 2021

@WouterJD

If you would be sure about this, try to use -p90 which adjusts the power-calculation (limited to 90...110%).

Will test this feature later next week

Question: I see you have the headunit out-of-reach; do you not use it and if not why not?
The headunit can be used to change gears: up/down for the rear; cancel for front and enter to reset.

I finally took the time to read isssue 120 to understand how this virtual gear-box works.
I tested this feature this morning. I noticed right away several benefits (less noise, smootheness, etc) I accidently went outside the cassette range a few times. I was to mention it but I can read above comments to that effet.
I wish I had the skills to program my own cassette with the -T feature and make it behave like my DI2 (11-28). In the meantime, I will just be careful with the buttons on my T1932 and make sure that I do NOT hit the DI2 shifters accidently. Difficult to change old habits

Many thanks.

@switchabl
Copy link
Contributor

I didn't get very far with my original implementation for the configuration file, but I have spent quite a bit of time thinking about the conceptual questions. I am really sorry that I didn't get around to writing this up earlier! But maybe you still find my thoughts useful.

I believe the configuration file will end up being more than just a way to change command-line arguments from the GUI. Things I can imagine right now:

  • people who run without GUI (e.g. on the Raspberry Pi) can still use it to set options conveniently, but will edit it by hand
  • we can have additional options that are not available on the command-line because they are rarely used or because they would be too big (like a keymap)
  • there may be options for more advanced users/debugging that are not available in the settings GUI, but can be edited manually (like power curve parameters, device IDs etc)
  • we can preserve state from one session to the next (like calibration values, information on paired devices)
  • and maybe more

That doesn't mean all of that needs to be implemented right now, but if we keep it in mind, we can hopefully avoid breaking changes later on:

  • In my opinion, order of precedence should be: defaults < config file < command-line arguments
    Most settings will be fairly permanent and will be set in the config file once. But some you may want to override occasionally on start-up without keeping them (like '-m').

  • This may be the best moment to clean up the command-line arguments since most people will never need to use them again afterwards. Maybe even something as simple as this

--autostart, --no-autostart
--calibrate, --no-calibrate
--config [file.json]
--debug [level]
--mode={ant,ble,power,grade,resistance}
--gui, --no-gui
--priority={default,power}
--tcx, --no-tcx

and keep everything else in the config file. Possibly a bit too radical, but you get the idea. The yes/no pairs will allow you to override every setting individually. Right now if you specify one option you have to specify all which is annoying.

  • Config file location:

    • There are OS specific locations for configuration files (e.g. ~/.config/FortiusANT/ on Linux). The appdirs package can be used to find the correct one (https://pypi.org/project/appdirs/). It is a good idea to use those by default because at one point we will probably want proper Linux packages and maybe a Windows installer and that just doesn't work properly if you write files in the executable/working directory.
    • For convenience, checking the working directory for an already-existing config file as well might be an option
    • Eventually there should be a command-line argument for the config file path. That will be convenient for those Raspberry Pi auto-start setups and maybe also if you have several config files that you want to switch between.
  • File format: The structure of the json file doesn't have to be the same as the current command-line options. We have the opportunity to sort the options into categories and choose nice descriptive names. To give you a general idea (probably missed something):

{
    "general": {
        "autostart": true,
        "debug": 0,
        "mode": "ant",
        "gui": true,
        "hrm": "ant",
        "record tcx": false
    },
    "trainer": {
        "type": "auto",
        "auto calibrate": true,
        "calibration value": 15,
        "power factor": 100,
        "pedal stroke analysis": true,
        "runoff": {
            "max speed": 40,
            "min speed": 1,
            "dip": 2,
            "power": 100,
            "time": 7
        }
    },
    "simulation": {
        "grade factor": 100,
        "downhill factor": 100,
        "crankset": [
            34,
            50
        ],
        "cassette": [
            34,
            30,
            27,
            25,
            23,
            21,
            19,
            17,
            15,
            13,
            11
        ],
        "start gear": [
            50,
            19
        ]
    },
    "ant": {
        "dongle id": 0,
        "priority": "power",
        "scs": false
    }
}
  • Saving the json file with something like sort_keys=True, indent=4 makes it easier to edit by hand

  • We will want a way to read the json file without importing wx (for command-line mode), so the settings GUI will probably need to go in a different file. We may also want to write some settings (in particular the calibration value) outside of the settings GUI, but I guess we can deal with that later.

@WouterJD
Copy link
Owner Author

WouterJD commented Jan 17, 2021

Good points, to be saved as a separate issue as feature-request.

There were two requirements

  • create user interface for settings
  • save settings in json file

both requirements are implemented.

The sort and indent is easily to be added; I will check later this week.

The priority is done like this, because: if you start FortiusAnt with command-line parameters, then change them interactively and save in the json-file - you should start without command-line parameters to quarantee the settings from the json-file to be in effect.

I would think somebody choses command-line OR json-file; to be documented.

Regarding the format, it could have been done as you say (looks good). Now it matches the documentation and fulfills the requirements. I keep it as is; otherwise 5.1 release would be delayed further, changing the format would require implementation- and test-time.
And for the non-command-line-users, the format of the json-file is irrelevant since they will use the inteactive settings.
For the ocmmand-line-users, it's all the same.

@switchabl
Copy link
Contributor

Alas, I sometimes struggle to keep up with the progress made here (surely a good thing!). I can understand you don't want to delay the release unnecessarily. And I think it will work fine (and be a big improvement) for most users. Almost everyone will use the interactive configuration. In fact I think you should put something like "don't use the command-line options unless you know what you are doing and have a good reason and don't mix both" in the manual.

Most of my suggestions are really about making life nicer for the Linux/Raspberry PI people. We command-line people do appreciate a clear and readable config file as much as anybody! Maybe I will work on a PR myself at a later point.

@jujroy
Copy link

jujroy commented Jan 17, 2021

Alas, I sometimes struggle to keep up with the progress made here (surely a good thing!). I can understand you don't want to delay the release unnecessarily. And I think it will work fine (and be a big improvement) for most users. Almost everyone will use the interactive configuration. In fact I think you should put something like "don't use the command-line options unless you know what you are doing and have a good reason and don't mix both" in the manual.

Most of my suggestions are really about making life nicer for the Linux/Raspberry PI people. We command-line people do appreciate a clear and readable config file as much as anybody! Maybe I will work on a PR myself at a later point.

Could not aggree more. Being non-technical, I love simplicity. Thanks

@WouterJD
Copy link
Owner Author

@switchabl you deserve additional credits for all the hard work; json file redefined. I will publish shortly :-)

@WouterJD
Copy link
Owner Author

Summarized on the latest remarks

  • config file is in the current directory.

  • precedence: defaults < commandline < configfile

  • json format is as requested

  • existing parameters are not removed; write json will overwrite and/or modify

  • no functional changes (at this moment) to avoid more complexity and/or incompatibility

  • json file reading is (should be) wx-independant (UseGui = False)

@switchabl
Copy link
Contributor

switchabl commented Jan 18, 2021

Oh wow, did not expect that. That sounds good to me. Things like config file location can be easily tweaked later on without breaking compatibility. Changing the file format later is possible but more annoying, so it is good to decide now.

I think you are probably right about the command line arguments: it is better not to make any incompatible changes right now. After some time, most people will probably use the config file anyway. Then we can think about how to redesign the command-line to make it more useful for advanced users without messing things up for anyone else.

@WouterJD
Copy link
Owner Author

Fortius Antifier v5.1 test V is published; see wiki for additional info

@Sat2Freak
Copy link

Hi Wouter.

Tested the latest version (Version 5.1 Test VI) and I must admit I'm impressed. At first sight, I can't find any issues, and the problems I mentioned earlier are solved.
This one is the best I've ever tested ;)
A big thanks for all your work.

@WouterJD
Copy link
Owner Author

@Sat2Freak thanks. Time to finalize

@jujroy
Copy link

jujroy commented Jan 21, 2021

@WouterJD
@pcuttriss
Hello everyone. I tested the latest version (test VI) yesterday using my Fortius (Motor Brake 110 Volts) and Zwift. Using the configuration file, I inserted 130 in the -p Box. Calibration and startup process went with no error. I do not have a Power Meter to do a comparison. My feeling (legs, hill, shifting, etc) is much closer to reality with this version and the 130 setting. For the first time in six weeks, I had to do down to my 34 ring in a 10% climb. My FTP went down as expected. Also worth mentionning that this time, I only used the virtual gearbox. DI2 was left in 50/17 for the complete ride. I would appreciate if riders with Power Meters could give us an idea of the -p number that they use along with the results and trainersetup. Great work guys. Will continue riding with this version and -p (130) (https://www.strava.com/activities/4652438238)

@jujroy
Copy link

jujroy commented Jan 21, 2021

Should be available now

@WouterJD WouterJD merged commit ec1988d into master Jan 21, 2021
@WouterJD
Copy link
Owner Author

All changes now merged; further improvements in next versions.

@WouterJD WouterJD deleted the 5.1+4.2-Quality-upgrade branch January 21, 2021 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants