kanflo / opendps

Give your DPS5005 the upgrade it deserves
MIT License
897 stars 124 forks source link

Add a 'DPS Mode' Screen #166

Closed kohrar closed 3 years ago

kohrar commented 5 years ago

I added a DPS mode screen which makes the display show the voltage, current, and power. I wanted it to look somewhat like the old firmware as that's what I'm used to.

A custom event handler to the screens in order to implement the ability to focus on the voltage/current numbers within the screen. You should be able to see this change with the first commit on this pull request. This should allow for screens to have special button behaviors if desired in the future.

Features:

dps mode

Issues: I was unable to replicate generating the large meter font on my system using the bundled Ubuntu font. As a consequence, the meter fonts will look slightly different. Perhaps Johan (or someone?) can regenerate the large meter font with the 'W' character.

kanflo commented 5 years ago

Nice! I tried regenerating the fonts:

region `rom' overflowed by 76 bytes

Bummer. Guess I have to look into optimisations first...

kohrar commented 5 years ago

I worked a little bit more on this dpsmode screen over the weekend and added a timer, a watt-hour meter, and a over power protection feature. The 3rd row that normally displays the power output can be changed to show these properties and can be changed using the set+(up/down) keys normally.

The 3rd row can also be changed by using the M1 + dial combination (similar to changing screens).

Eg. pics pics

I have not pushed these changes to the branch on this pull request though since I needed to change even more things with the event system and modified how existing uui-elements work which might be too intrusive for your project. Feel free to review the code changes I made at https://github.com/kohrar/opendps/tree/func_dpsmode_timer_meter and let me know what you think.

As part of my development process, I've been using NerdFont as my typeface (https://github.com/kohrar/opendps/tree/nerdfont). As a consequence, this probably means none of these changes will look great with the Ubuntu font that's being used.

Xenoamor commented 5 years ago

Nice! I tried regenerating the fonts:

region `rom' overflowed by 76 bytes

Bummer. Guess I have to look into optimisations first...

image Not sure if any of these are fit for being cut. We might want to used fixed point instead of floating as it'll be faster and we'll save a decent chunk of ROM

732 bytes can be freed by not using (u)int64_t divisions

See #176 for space optimisation of the protocol handler

kanflo commented 5 years ago

Impressive @kohrar! I think the battery charging folks will appreciate your work and I would love to see it on master. I tried building your branch but I seem to be about 3kb short of flash. Which version of gcc do you use?

@Xenoamor : thanks for the insight that my thermometer occupies a whopping 3kb!

kohrar commented 5 years ago

@kanflo I'm compiling OpenDPS with only the function generator and the screen in this PR which probably saves a lot of space. I'm also compiling with gcc version: arm-none-eabi-gcc (15:7-2018-q2-6) 7.3.1 20180622 (release) [ARM/embedded-7-branch revision 261907]

I've made a few more changes in yet another branch because I can't help but add more features that I somewhat miss with the original firmware. https://github.com/kohrar/opendps/tree/func_dpsmode_recall_brightness.

Changes this time around:

recall-brightness

I also ran out of ROM space as well and had to remove the thermometer graphics. I'll spend the next few days cleaning up and optimizing the code.

There has been quite a lot of changes at this point which might make a pull request a little tricky. Are you comfortable having this PR contain some changes to the UI system? Mainly, the addition of new event types and logic in events.h, hw.c, uui.c. See: https://github.com/kohrar/opendps/compare/master...kohrar:func_dpsmode_recall_brightness?expand=1

Xenoamor commented 5 years ago

I'm okay with the changes. I'll have a play around with this PR when I get the chance but things like the brightness and memory recall seem very useful. I see no downside to having more events either.

Ideally I'd like to make a settings screen and have the brightness on there this would help to reduce any clutter. What really drew me to OpenDPS in the first place was how much simpler the display was

kanflo commented 5 years ago

I agree about the settings screen @Xenoamor. @kohrar would you mind to add instructions to your screen in eg. func_dpsmode.c? When this screen goes to master and the OpenDPS wiki gets written I am thinking each screen should have instructions.

tzarc commented 5 years ago

Maybe we revisit using FontAwesome etc. for the temp/power/wifi icons etc, given that the font renderer can do colours and the resulting glyphs will be significantly smaller than bgr565?

kohrar commented 5 years ago

IMG_20191005_175618 I've merged all my other branches into this PR.

This branch contains the following features and changes:

Documentation for the supported features can be found on my wiki at: https://leo.leung.xyz/wiki/DPS5005#DPS_Mode_Screen

This screen expects the display to be clear on activation as explained in https://github.com/kanflo/opendps/pull/165 and https://github.com/kanflo/opendps/pull/171. Without these PRs, you will see visual artifacts.

At this point, I believe this branch is stable and no longer WIP. Please let me know if you find bugs or issues.

kanflo commented 5 years ago

Nice work! I found one glitch, when output is enabled and the mode is changed the output should be disabled before switching mode. When testing I noted that this combination now seems to equal long press of ROT. Incidentally, two long presses are needed to get out of the lock mode.

I also get a few warnings I need to look into.

kanflo commented 5 years ago

I also noted that the font size has been reduced slightly. Not an issue but it does make the other modes look a bit "sparse". I guess the UI code need some adaptation wrt. font sizes.

kohrar commented 5 years ago

I found one glitch, when output is enabled and the mode is changed the output should be disabled before switching mode. When testing I noted that this combination now seems to equal long press of ROT. Incidentally, two long presses are needed to get out of the lock mode.

It slipped my mind that I added this. It isn't a bug per se, but was intentionally added since I did not want to accidentally change the mode (ie. the screen) and thereby killing the output and whatever I was powering. Due to the way the lock icon is implemented, I was unable to temporarily show the lock icon which was what I intended to do https://github.com/kanflo/opendps/blob/85c3e6a34ca80a4f24cebde2a925dd68974efdc2/opendps/opendps.c#L542-L550

When you attempt to change the mode while output is enabled, the lock icon will appear as if you turned lock mode on by holding ROT. However, it isn't actually in lock mode and you will still be able to change modes after output is disabled. You would need to lock and unlock in order to clear the lock icon though (as you've discovered).

Since I have a settings screen created in another PR, perhaps behaviors like this can be adjusted rather than compiled in as compile-time options.

In the mean time, I will remove this behavior from this PR.

kohrar commented 5 years ago

I also get a few warnings I need to look into.

I've done some clean up and fixed the incorrect event prototype. You should no longer be getting any warnings.

gagarcr commented 4 years ago

Any update on this? I would love to have it merged!

godoto commented 4 years ago

Its stable? Can be merged?

kohrar commented 4 years ago

I've been using this feature on my two DPS5005s without any issue. It's up to @kanflo to merge at this point.

If you want to try it out, you can checkout this branch and test it. Please report any issues if you do!

kanflo commented 4 years ago

It has been quite a while but I will check this PR again the coming week and hopefully merge it. Sorry for the delay.

Steamerzone commented 4 years ago

I've been using this feature on my two DPS5005s without any issue. It's up to @kanflo to merge at this point.

If you want to try it out, you can checkout this branch and test it. Please report any issues if you do!

@kohrar

Do you have a branch available with the DPS_Mode (preferred as a default screen) and the screen clear fixes?

danielkucera commented 4 years ago

THIS LOOKS GREAT! Can we have it please? :)