cagnulein / qdomyos-zwift

Zwift bridge for smart treadmills and bike/cyclette
https://www.qzfitness.com/
GNU General Public License v3.0
415 stars 116 forks source link

Kingsmith walkingpad X21, app not responding #1031

Closed ChiTat73 closed 1 year ago

ChiTat73 commented 2 years ago

Describe the bug Kingsmith walkingpad X21, app not responding, app not displaying speed or control treadmill speed.

To Reproduce Steps to reproduce the behavior:

  1. Go to 'QZ app' 2 setting kingSmith option -> WalkingPad X21 V2 3 restart app
  2. app finds kingsmith walkpad X21
  3. Start X21 treadmill manual
  4. app show to readings of speed or any control

Expected behavior expected speed reading of treadmill or able to control speed from app

Screenshots unnamed unnamed

Smartphone (please complete the following information):

Append a debug log

Follow this guide https://github.com/cagnulein/qdomyos-zwift/wiki/How-do-i-get-the-debug-log-in-case-something-doesn't-work%3F

Additional context Add any other context about the problem here.

cagnulein commented 2 years ago

hi @ChiTat73 but i don't see the debug logs. could you please upload them?

ChiTat73 commented 2 years ago

just done it now subject = Kingsmith walkingpad X21, app not responding

On Tue, Nov 8, 2022 at 8:12 PM Roberto Viola @.***> wrote:

hi @ChiTat73 https://github.com/ChiTat73 but i don't see the debug logs. could you please upload them?

— Reply to this email directly, view it on GitHub https://github.com/cagnulein/qdomyos-zwift/issues/1031#issuecomment-1307112128, or unsubscribe https://github.com/notifications/unsubscribe-auth/A4CHRF4YMWR5HEQLPTRFOPTWHI7RTANCNFSM6AAAAAAR2H23FM . You are receiving this because you were mentioned.Message ID: @.***>

cagnulein commented 2 years ago

@ChiTat73 ok but I still need the debug log, please attach it from the github web interface Thanks

ChiTat73 commented 2 years ago

debug-Tue_Nov_8_21_06_11_2022.log

cagnulein commented 2 years ago

does the app crashes or just freeze in the same window? in the log there is just the attempt to the connection and nothing else, very strange

ChiTat73 commented 2 years ago

I think the app is running as I see the Heart rate change in the app from my apple watch.

cagnulein commented 2 years ago

hah ok so you should have other log, because this one it seems truncated. Check in the same folder and upload all of them. Thanks!

ChiTat73 commented 2 years ago

debug-Tue_Nov_8_21_06_11_2022 (2).log debug-Tue_Nov_8_20_56_58_2022.log debug-Tue_Nov_8_21_05_01_2022.log debug-Tue_Nov_8_21_04_31_2022.log

Tue Nov 8 21_05_56 2022_heart

cagnulein commented 2 years ago

@d3m3vilurr I guess we're facing a new version of the Kingsmith, could you please check it if you see something familiar? Thanks!

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

d3m3vilurr commented 1 year ago

@cagnulein Sorry, I just read this issue now. I'll check it tonight.

d3m3vilurr commented 1 year ago

ZaCw4FGHIJqLhNYP9RVTU/Wc+6ObDdefgEijklmnopQrsBuvMxXz1yA2t5078KS3= might be the key. (swap + <-> Y from original table)

cagnulein commented 1 year ago

@d3m3vilurr thanks! I will give it a try!

d3m3vilurr commented 1 year ago

@cagnulein IMO, keys could be related by device id.

if this idea is right, we are able to reduce the on/off flag to select the key using inheriting device class (or using another method for it)

800 shown these 3 key type can handle 75% of device ids. and i'm not sure that KS-X21 is real device id or not.

(kingsmith can add new device id into the database. but i don't check newer device table of current kingsmith application yet..)

cagnulein commented 1 year ago

interesting @d3m3vilurr i didn't notice this. We need some confirmations from the user to be sure about this

d3m3vilurr commented 1 year ago

right. but idk how many ks x21 users use the qz app for zwift. do you use telemetry library or others? if we can summary x21 device id + key pair, confirming could be easy.

cagnulein commented 1 year ago

right. but idk how many ks x21 users use the qz app for zwift.

idk unfortunately. but i received at least 20 mails about different users

do you use telemetry library or others?

nope, i don't track users.

if we can summary x21 device id + key pair, confirming could be easy.

I will ask to the users that contact me by email to send me a debug log to confirm you your idea. Thanks!

d3m3vilurr commented 1 year ago

I love that you don't track any user's data 👍 and I think 20 users are pretty enough to confirm idea :)

cagnulein commented 1 year ago

as soon as a user will write me about the X21 I will reply back to him with the request of a debug log in order to clarify this behaviour :)

cagnulein commented 1 year ago

@d3m3vilurr unfortunately the new key seems that it's not ok for the OP. Check the log debug-Tue_Dec_6_18_41_13_2022.txt

d3m3vilurr commented 1 year ago

I'll recheck this. debug-Tue_Nov_8_21_06_11_2022.2 showns servers getProp 1 2 7 12 23 24 31 is still returned error. but some props can receive from device. but new key seems to have trouble to send request. (but i think pretty close to find a new key :))

d3m3vilurr commented 1 year ago

can you retry with ZaCw4FGHIJqLhN9P+RVTU/WcY6ObDdefgEijklmnopQrsBuvMxXz1yA2t5078KS3=? swapped 9<->+

it can decrypt props runState 0 CurrentSpeec�0.0 RunningTotalTime 73 of debug-Tue_Nov_8_21_06_11_2022.2

cagnulein commented 1 year ago

done @d3m3vilurr thanks! I will send the new version to @ChiTat73 today/tomorrow!

cagnulein commented 1 year ago

@d3m3vilurr great! the new key is working, but

the start stop button on the app doesn’t work. But if I start the treadmill first then the app will work using the speed + and - this will control the treadmill.

is something similar also with your treadmill? debug-Fri_Dec_9_18_25_48_2022.txt

d3m3vilurr commented 1 year ago

yes. #813 has a part for controlling the power & running mode

an application have to send props ControlMode 1 to turn on the treadmil then process has to send props runState 1 to start up the treadmil.

ControlMode has 3 options;

runState has 2 options;

cagnulein commented 1 year ago

do you think that #813 could be merged or could lead to new issues? @d3m3vilurr

d3m3vilurr commented 1 year ago

i think #813 is not perfect. just reduce the problem..(and my code is not clean & outdated ;) ) a qz application has to update application's state machine (idk current codeset anyway) to receive / sync from BT devices.

main problem points are;

these two reason, an application can have different state between treadmils. so my idea is

@cagnulein what is your main treadmil? and doesn't it have similar issue?

cagnulein commented 1 year ago

@d3m3vilurr my main treadmill is a domyos intense run and it doesn't have any state or something. I mean the speed is 0 when i't stopped and more than 0 when it's running. Very simple :)

So It's hard for me to understand all this state machines :D I mean I can see the meaning, but I don't know how actually implement this.

Just to understand: you will need a popup or similar in the UI that the user should press in order to go to the next state machine step? Of course it's doable. I just need to know the detail of the signal and slot that we have to emit.

Do you think it's a nice to have or it's very important for walkingpad users? I mean I guess QZ has been used from a lot of Walkingpad users but only you and the OP asked me about this :D

d3m3vilurr commented 1 year ago

well... yes... i thought we need it for users. (and also ks code can contain some of my faults if only ks users have a problem :))

KS device support two mode that is manual & automatic. maybe automatic mode is similar as traditional treadmill. it's very simple. speed 0 is stop >0 is walk. but this mode cannot go to >6. only manual mode can set 6 < speed <=12.

i think, modern treadmill can have own state. the companion application is just handling and helping the update the device state.

idk it's right or not.

  1. treadmill class should have only one event loop to prevent multiple calling the writeCharacteristic.
  2. qz application have state machine on the UI part. but sending message doesn't mean applied on device. sometimes message can be ignoreable. for example, when sent the command to KS treadmill, device will return the result of the command. but this packets will be splitted and you have to wait multiple sec to receiving the full message. so if you press the button multiple times, device will ignore next command until sending every result packet snippets.

imo & already you knows, second reason is more important. UI state has to be always updated by changing result of device state. but (in my old memory) when i press the stop(it's not only one. change speed button is same), application will stop immediatly even if treadmil is not finished the stop process...

so, my point is... popup or dimmed or disabling the button is not perfact solution. current KS device may need method to handle the UI state directly... that this reason, #813 has these thing on the patch set

  1. bt device has a method for following the state machine of the device side.
  2. homeform has event to receive data from device side for updating UI.

anyway my old patchset is too complex and outdated now. and also it's out of ks key issues.

maybe we need an another issue for it :)

cagnulein commented 1 year ago

KS device support two mode that is manual & automatic. maybe automatic mode is similar as traditional treadmill. it's very simple. speed 0 is stop >0 is walk. but this mode cannot go to >6. only manual mode can set 6 < speed <=12.

ok, got it now.

i think, modern treadmill can have own state. the companion application is just handling and helping the update the device state.

mmm... as far as i know, this treadmill is the only one.

  1. treadmill class should have only one event loop to prevent multiple calling the writeCharacteristic.

yes and it has the perfect logic for that particular model of the treadmill

  1. qz application have state machine on the UI part. but sending message doesn't mean applied on device. sometimes message can be ignoreable.

yes

for example, when sent the command to KS treadmill, device will return the result of the command. but this packets will be splitted and you have to wait multiple sec to receiving the full message. so if you press the button multiple times, device will ignore next command until sending every result packet snippets.

that's not true for all the devices. i mean on my treadmill every action is immediate. so maybe the way to implement this, is adding a state machine inside the kingsmith class.

imo & already you knows, second reason is more important. UI state has to be always updated by changing result of device state. but (in my old memory) when i press the stop(it's not only one. change speed button is same), application will stop immediatly even if treadmil is not finished the stop process...

how long does it take to stop? I mean also my treadmill takes 3 seconds to a full stop from 10km/h but it's not a big deal, is it for this treadmill? for the speed buttons, instead, there is already a caching mechanism, and we can adopt for the inclination too if needed.

so, my point is... popup or dimmed or disabling the button is not perfact solution. current KS device may need method to handle the UI state directly... that this reason, #813 has these thing on the patch set

so you want to disable the start and stop button until a signal for the treadmill class is emitted?

maybe we need an another issue for it :)

let's stay with this, I mean, again, in my idea, this is strictly releated to this particular treadmill.

d3m3vilurr commented 1 year ago

i think, modern treadmill can have own state. the companion application is just handling and helping the update the device state.

mmm... as far as i know, this treadmill is the only one.

I mean, treadmils which uses electornic power can control on device itself. If you are able to control this device with two or more ways at same time(eg; official app, dashboard control and others), device have to decide own state. for example you can turn on with dashboard controller, then you can change the speed with qz app. in this case, app shown stop but dashboard is running.. in this reason, I said modern treadmills can have own state machine internally

it's totally same as KS case. but IDK it's only related with my implementation or not.

for example, when sent the command to KS treadmill, device will return the result of the command. but this packets will be splitted and you have to wait multiple sec to receiving the full message. so if you press the button multiple times, device will ignore next command until sending every result packet snippets.

that's not true for all the devices. i mean on my treadmill every action is immediate. so maybe the way to implement this, is adding a state machine inside the kingsmith class.

yes. you right. most treadmills can communicate with small command code. but this device group uses encrypted text command. so this device needs more bytes and time for communication.

imo & already you knows, second reason is more important. UI state has to be always updated by changing result of device state. but (in my old memory) when i press the stop(it's not only one. change speed button is same), application will stop immediatly even if treadmil is not finished the stop process...

how long does it take to stop? I mean also my treadmill takes 3 seconds to a full stop from 10km/h but it's not a big deal, is it for this treadmill?

I couldn't remember. just need few seconds. but decresing the speed to 0 is not meant stop. minimum speed might be 2km/h and device have control mode to stop (standby mode)

these are idea steps when handshake

  1. check the current state datas
  2. if device is activated, turn off device with standby mode or update current state data

when press start

  1. turn on with manual mode (or automatic mode. but automatic mode cannot use changing speed feature)
  2. an app have to sleep until receiving turn on message
  3. speed of treadmills will increase 2km/h automatically
  4. now you can change speed

when press pause

  1. turn off with standby mode
  2. app have to wait until finishing this command

when press stop

  1. turn off with standby mode
  2. app have to wait until finishing this command

for the speed buttons, instead, there is already a caching mechanism, and we can adopt for the inclination too if needed.

well.... in my expectation, a cache is not perfact. it's just expectation so idk.

so, my point is... popup or dimmed or disabling the button is not perfact solution. current KS device may need method to handle the UI state directly... that this reason, #813 has these thing on the patch set

so you want to disable the start and stop button until a signal for the treadmill class is emitted?

yes. actually not only butten. imo, speed meter and others also have to be controled by treadmill device class

maybe we need an another issue for it :)

let's stay with this, I mean, again, in my idea, this is strictly releated to this particular treadmill.

yes. you right.


sorry my eng. it's hard to explain my case :) maybe if device class can have one event loop, and we can ignore middle of messages, we just need to focus the handshake case.

cagnulein commented 1 year ago

I couldn't remember. just need few seconds. but decresing the speed to 0 is not meant stop. minimum speed might be 2km/h and device have control mode to stop (standby mode)

hah ok, so here it is the confusion. On my idea stop means 0 km/h, but it's not true for this treadmill :)

when press start

  1. turn on with manual mode (or automatic mode. but automatic mode cannot use changing speed feature)
  2. an app have to sleep until receiving turn on message
  3. speed of treadmills will increase 2km/h automatically
  4. now you can change speed

what I don't understand is: what does it happen on the current implementation if you press something during this "sleep time"? I mean if the treadmill just rejects the frames, I guess we are good to go also with the current implementation, am I wrong? I mean I don't want to over complicate the things :)

d3m3vilurr commented 1 year ago

when press start

  1. turn on with manual mode (or automatic mode. but automatic mode cannot use changing speed feature)
  2. an app have to sleep until receiving turn on message
  3. speed of treadmills will increase 2km/h automatically
  4. now you can change speed

what I don't understand is: what does it happen on the current implementation if you press something during this "sleep time"? I mean if the treadmill just rejects the frames, I guess we are good to go also with the current implementation, am I wrong? I mean I don't want to over complicate the things :)

Current implementation doens't have a part to send ControlMode 1 to change mode to manual And sorry I found my one mistake of my explaination, A device has a command of start and stop it's props runState 1 and props runState 0

https://github.com/cagnulein/qdomyos-zwift/pull/813/files#diff-2ae9b74d7c341fe601d1f86f2c8994c7c7d3db5c30f7d51d0eb1a7764366c785R195-R201

This part is what have to do when after handshake. it shown sending changing mode first then sending start

My sleep mean it should wait finish the command. So, I think you are right, waiting doesn't an important when changing speeds. But it's still important when handshake timing or start the exercise. If device is not in manual mode & it doesn't be started yet, it'll ignore every next commands. (cuz it's stopped)

cagnulein commented 1 year ago

If device is not in manual mode & it doesn't be started yet, it'll ignore every next commands. (cuz it's stopped)

got it. But we can't just put a command queue in the kingsmith class? It will be easier to implement and it will have the same behaviour, am I wrong?

d3m3vilurr commented 1 year ago

no, you right. that is one of easy solution. just thing is, device is still controlable by user's action of remote control or dashboard. in this time, device can have different state.

in this reason, I want a synchronize method between device and app. :)

here is one of my fault scenarios.(just old memory so idk it's true or not...)

  1. start in qz
  2. speed up using remote, app or others
  3. pause in qz
  4. start using remote controller

if we don't sync state between device and app, we might find various weird cases... for example here

  1. handshake the app; (if we update handshake step to turn on the device in here)
  2. change mode automatic or others using remote controller (because if device was folded handle bar, it only can enable automatic mode. it mean, app's sending manual mode code will be ingored) (so we can think, fold handle bar in this time)
  3. start

technically, we don't need to check automatic or manual mode. :p because speed will calculate automatically.. just can't control speed in automatic mode.

real problem is just mismatched runState and controlMode. and these two call have to wait to apply in device.


recently i didn't run on KS pad. (just my gf still use it but she doesn't use KS app & QZ app) so i just explained with my old memory and in this reason, my explaination can have mistake part. sorry about this.

so i agree that just adding the command chain into some steps could reduce(or resolve) the problem. but when i made the pr #813, i thought that this issue was required synchronize method. and still I think of it. i might be wrong, so i think just approch step by step is better.

how about just adding the command chain into handshake and start/stop timing for first step? if we still get a problem after that, we'll get more clear situation and find more good solution of it.

cagnulein commented 1 year ago

how about just adding the command chain into handshake and start/stop timing for first step? if we still get a problem after that, we'll get more clear situation and find more good solution of it.

yeah sure! so if i'm right we could use the #813 , splitting the modifications and using only the kingsmith class modifications and adding a queue there, is it correct?

d3m3vilurr commented 1 year ago

yes

cagnulein commented 1 year ago

@d3m3vilurr https://github.com/cagnulein/qdomyos-zwift/pull/1082 did a simplify everything too much? If the runstate is not in start mode i'm caching the speed and inclination force commands (if any in the queue).

d3m3vilurr commented 1 year ago

@cagnulein looks good to me #1082. i'm worried some weird cases, but we can ignore it until actually being happened.

cagnulein commented 1 year ago

@d3m3vilurr are you able to try it before merging it?

d3m3vilurr commented 1 year ago

sorry i can't test it til next week. :'( can you wait next monday or tuesday?

cagnulein commented 1 year ago

no problem :)

Il giorno sab 17 dic 2022 alle 08:28 Sunguk Lee @.***> ha scritto:

sorry i can't test it til next week. :'(

— Reply to this email directly, view it on GitHub https://github.com/cagnulein/qdomyos-zwift/issues/1031#issuecomment-1356087700, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALYWFGGZIKOWYC3EP2ORTWNVTQRANCNFSM6AAAAAAR2H23FM . You are receiving this because you were mentioned.Message ID: @.***>

-- Roberto Viola Software engineer and open source enthusiast http://robertoviola.cloud

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.