laberning / openrowingmonitor

A free and open source performance monitor for rowing machines
https://laberning.github.io/openrowingmonitor
GNU General Public License v3.0
98 stars 19 forks source link

Give the user the option on which field to overwrite with heart rate data #131

Open sean808080 opened 1 year ago

sean808080 commented 1 year ago

When no heart rate strap is nearby, the default fields include number of strokes. When a heart rate strap is powered on and picked up by ORW, the number of strokes field is removed and the heart rate data takes its place.

Can the option given to the user as to what data field is overwritten with the heart rate data? Perhaps a configuration can be setup for this. I would prefer watts be overwritten by the heart rate data perrsonally.

Thanks for this wonderful project. I am very excited to continue my exploration here.

beavis69 commented 1 year ago

I'd like to have that option too, or a way to use a custom html/css template.

beavis69 commented 1 year ago

Actually with a tiny 3.5" rpi screen, i'd like to have the icons removed and bigger text box and font.

Abasz commented 1 year ago

I've implemented the above requested feature: https://github.com/laberning/openrowingmonitor/discussions/126#discussioncomment-5393745 You can find the details in this discussion. Would you be willing to test?

What needs to be done is to switch to my repo, fetch the branch install the update and build the new gui. This can be done with the below code sequence:

git remote add devel https://github.com/Abasz/openrowingmonitor.git
git fetch --all
git checkout devel/improve-gui
npm install
npm run build
sean808080 commented 1 year ago

hey @Abasz Thanks for sharing your enhancement. It took me a while to get to it.

First thoughts are it is FANTASTIC! It's so nice to have control over the data fields and ordering of them in the UI.

IMG_1554

Thanks again for pulling this off! Only a minor bug which I am sure you already know is the force curve shows 'undefined' when starting out.

Screenshot 2023-03-23 at 3 30 27 PM

sean808080 commented 1 year ago

Actually with a tiny 3.5" rpi screen, i'd like to have the icons removed and bigger text box and font.

this is now available with @Abasz 's enhancement further down this thread.

Abasz commented 1 year ago

I did not encounter this undefined bug. Can you please open devtools in the browser (in chrome and many other its F12, mobiles do not have it though) and see if there is any error message there?

beavis69 commented 1 year ago

I'll try it tomorow. Great work !

Another cool feature i would love to have is colored bpm value like polar flow does on android app, with 5 interval colors based on % of your max hr setting :

And to have the HR updated even when you pause (HRR-1/HRR-2 as bonus ?)

sean808080 commented 1 year ago

@Abasz That undefined error is shown consistently on a Mac running Safari prior to a workout. I am not sure how to get to the spot you suggested but here are a few screenshots stepping through errors in the debugger on Safari. Hope that helps. I don't see undefined when running firefox on safari.

Screenshot 2023-03-24 at 8 35 45 AM Screenshot 2023-03-24 at 8 36 21 AM Screenshot 2023-03-24 at 8 36 06 AM

Abasz commented 1 year ago

Thanks for the above, I meant errors in the console.

This will be hard to debug, I am not sure how I will do this: "Apple no longer offers Safari updates for Windows." :):)

The problem is that I am not able to reproduce the bug. Nevertheless, I made some updates (including code refactoring that targets variable safety as well)

Should you wish to update:

git pull devel improve-gui
npm install
npm run build
sean808080 commented 1 year ago

I pulled the update and still see the same undefined appearing in the force curve field.

here's a screenshot of the console...Not sure if there is anything useful there. Screenshot 2023-03-24 at 4 20 15 PM

sean808080 commented 1 year ago

@Abasz Should I try an update? i"m not sure there is a new version to update to.

Abasz commented 1 year ago

@Abasz Should I try an update? i"m not sure there is a new version to update to.

Sure feel free, but I have not made any changes since I started the PR (approx 2 week ago). I was not able to reproduce the "Undefined" problem, but should fix battery indicator and bring a fully functional version to the metric selection (with added metrics).

sean808080 commented 1 year ago

ok for the update I had to do a stash first bec. I made an edit so to update correctly

git stash
git pull devel improve-gui
npm install
npm run build

now the UI is refreshed. One thing is it throws the following error when I toggle the BT heart sensor on/off 3 times:

Error: Error opening Ant Stick
Apr 08 11:37:39 rowingmonitor npm[5293]:     at AntManager.openAntStick (file:///opt/openrowingmonitor/app/peripherals/ant/AntManager.js:21:47)
Apr 08 11:37:39 rowingmonitor npm[5293]:     at processTicksAndRejections (node:internal/process/task_queues:96:5)
Apr 08 11:37:39 rowingmonitor npm[5293]:     at async EventEmitter.attach (file:///opt/openrowingmonitor/app/peripherals/ant/HrmPeripheral.js:19:38)
Apr 08 11:37:39 rowingmonitor npm[5293]:     at async createHrmPeripheral (file:///opt/openrowingmonitor/app/peripherals/PeripheralManager.js:231:11)
Abasz commented 1 year ago

ok for the update I had to do a stash first bec. I made an edit so to update correctly

git stash
git pull devel improve-gui
npm install
npm run build

now the UI is refreshed. One thing is it throws the following error when I toggle the BT heart sensor on/off 3 times:

Error: Error opening Ant Stick
Apr 08 11:37:39 rowingmonitor npm[5293]:     at AntManager.openAntStick (file:///opt/openrowingmonitor/app/peripherals/ant/AntManager.js:21:47)
Apr 08 11:37:39 rowingmonitor npm[5293]:     at processTicksAndRejections (node:internal/process/task_queues:96:5)
Apr 08 11:37:39 rowingmonitor npm[5293]:     at async EventEmitter.attach (file:///opt/openrowingmonitor/app/peripherals/ant/HrmPeripheral.js:19:38)
Apr 08 11:37:39 rowingmonitor npm[5293]:     at async createHrmPeripheral (file:///opt/openrowingmonitor/app/peripherals/PeripheralManager.js:231:11)

Thanks for the feedback. The error relates to the Ant stick handling. Do you have one plugged in?

EDIT:

This is actually not a new error, the code that generates the error has been around since quite some time. From a design perspective it may not have been the best choice to throw an error when opening the Ant stick fails as it could be for numerous reasons. But this was the way it has been originally implemented so I did not touch it.

Probably logging an error and providing more context (like "may be no ant+ stick is plugged in") is a better choice. The problem really is that non of the ant+ libraries currently exposes real reason that is behind the failure of connecting to the stick (i.e. whether it is busy, not connected, communication error occurs etc.). Actually looking at the code I suppose it would be possible to fork that project and add an enum return that would tell more about the reasons for not being able to connect.

sean808080 commented 1 year ago

Hey @Abasz just a quick note to say I'm loving the updates. The ability to have 12 fields on my iPad makes ORM fantastic! Thanks for sharing your updates! IMG_1561

Abasz commented 1 year ago

FYI - The improved GUI PR was merged into Jaap's v1beta_update branch. This should mean that I recommend switching back to that branch on his repo.