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

feat: Charging status #50

Merged
merged 2 commits into from
Oct 4, 2024
Merged

feat: Charging status #50

merged 2 commits into from
Oct 4, 2024

Conversation

kienvo
Copy link
Member

@kienvo kienvo commented Sep 11, 2024

Resolves #39.

Summary by Sourcery

Implement a charging status feature that includes battery level detection and display, along with a new font rendering capability for text display on the device.

New Features:

  • Introduce a charging status feature that detects and displays the battery charging state and percentage on the device.

Enhancements:

  • Add a new font rendering capability to display text on the device's framebuffer using a 5x7 font.

@froscon2024
Copy link
Contributor

froscon2024 commented Sep 23, 2024

Why did you add the coffee cup again? Yes, it was there before in the closed source firmware, but is the coffee cup something that should be there? Did you liked the design of the coffee cup?
I would prefer something else next to the battery symbol so that you can see a difference between the open and the closed firmware when they are both in this state. Maybe write "FOSSASIA" and the firmware version there instead of a useless coffee cup. Or at least only the firmware version next to the battery symbol.

Also one thing i did not liked in the closed source firmware was that it was not possible to charge the badge and still use it. I had either the tag-info without charger attached or coffee cup when charger attached.
Adding the option to charge while still displaying the information would be the first benefit of the FOSS firmware compared to the closed source one.

- add 5x7 font
- add animation
- display version after the animation end
- add BOOT state for switching to normal mode while charging
@kienvo kienvo force-pushed the batt-indicator branch 2 times, most recently from 313aced to 907bf59 Compare September 29, 2024 02:47
@kienvo
Copy link
Member Author

kienvo commented Sep 29, 2024

The coffee cup was added because of my laziness. I had no idea instead of just made it like the closed firmware. Now removed.

Placing the firmware version next to the battery symbol is a good idea. I've just added an animation that displays "FOSSASIA" -> "Badge Magic" -> version. The version number will automated in #53.
badgemagic charging preview

My badge comes with closed firmware that can be used while charging, by pressing the KEY2. Seem like your badge has a different firmware.

@kienvo kienvo changed the title feat: Battery indicator feat: Charging status Sep 29, 2024
@kienvo kienvo marked this pull request as ready for review September 29, 2024 03:36
Copy link

sourcery-ai bot commented Sep 29, 2024

Reviewer's Guide by Sourcery

This pull request implements a charging status feature for the device. It adds functionality to display battery status, detect charging, and show a boot animation. The changes primarily affect the main program flow, add new display functions, and introduce battery-related calculations.

Sequence Diagram

sequenceDiagram
    participant Main
    participant ChargingStatus
    participant Display
    participant Battery
    Main->>ChargingStatus: disp_charging()
    loop While in BOOT mode
        ChargingStatus->>Battery: read_batt_raw()
        Battery-->>ChargingStatus: battery raw value
        ChargingStatus->>Battery: bat_raw2percent()
        Battery-->>ChargingStatus: battery percentage
        ChargingStatus->>ChargingStatus: is_charging()
        alt Is Charging
            ChargingStatus->>Display: Show charging animation
            ChargingStatus->>Display: Show battery percentage
        else Not Charging
            ChargingStatus->>Display: Show static battery status
            ChargingStatus->>Main: Return to main loop
        end
    end
    Main->>Main: Continue with normal operation
Loading

File-Level Changes

Change Details Files
Implement charging status detection and display
  • Add new BOOT mode to the MODES enum
  • Configure CHARGE_STT_PIN for charging status detection
  • Implement functions to read battery voltage and calculate percentage
  • Add functions to display battery status and charging animation
  • Modify main loop to include charging status display during boot
src/main.c
Add new resources for battery and animation display
  • Include new XBM resources for battery icon, warning icon, and animation
  • Define new xbm_t structures for the added resources
src/resource.c
src/resource.h
Implement text rendering functionality
  • Add a 5x7 pixel font array
  • Implement functions to render individual characters and strings to the framebuffer
src/font.c
src/font.h
Update build configuration
  • Add src/font.c to the list of source files in the Makefile
Makefile

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @kienvo - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@@ -0,0 +1,100 @@
#include <stdint.h>

uint8_t font5x7[][6] = { //Font 5x7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Add a comment explaining the structure of the font array

A brief comment explaining how the bit patterns in the array correspond to character shapes would improve the readability and maintainability of this code.

// Font 5x7: Each character is represented by 6 bytes
// The first 5 bytes define the 5x7 pixel grid (columns)
// The 6th byte is always 0x00 (for spacing)
// 1 in a bit position means pixel on, 0 means pixel off
uint8_t font5x7[][6] = { //Font 5x7

@fcartegnie
Copy link
Collaborator

The coffee cup was added because of my laziness. I had no idea instead of just made it like the closed firmware. Now removed.

Can't we just make simple things ?
We don't need to write version nor advertise fossasia everywhere.

@froscon2024
Copy link
Contributor

Having the firmware version somewhere visible for a normal user without the need of additional hardware to read out would be beneficial. In general people always have devices these days that report a firmware version.
The closed source OEM firmware just had a single version and there was no changes planned. But now there are versions and probably feature addons and functionality differences between the versions. Letting the people know what version is installed is nice.

@fcartegnie fcartegnie merged commit bb33a6d into fossasia:master Oct 4, 2024
2 checks passed
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.

Charging status
3 participants