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.

Sign up for GitHub

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

Jump to bottom

Major Update #16

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Major Update #16

wants to merge 24 commits into from

Conversation

darkgrue
Copy link

@darkgrue darkgrue commented May 9, 2021

Major refactor and cleanup of code to address multiple functional and stylistic issues.

  • Update README.md
  • Stylistic cleanup
    • Rename functions to remove typos, provide better description of function
    • Formatting
    • Spelling errors
    • Addressed many typos
    • Reordered functions by order of call
  • Removed unneeded includes
  • Fixed -Wall compiler warnings
  • More robust error handling
  • Fixed the way top was being called
  • Added /proc/stat CPU calculation and enabled in favor of top (should be more accurate)
  • Added df disk free space calculation and enabled in favor of statfs() (should be more portable)
  • Refactored the way numbers were being pushed to the display that should result in better formatting
    • Fit decimals by width using significant figures
    • Right-justify fields (against labels)
  • Correctly display CPU temperature units of measurement (F displayed instead of C)
  • Added additional logic to find default interface to query for IP address to display.
  • Added /contrib directory, support scripts
    • script to tar up files
    • systemd service script

darkgrue added 12 commits May 1, 2021 19:36
@darkgrue
Update README.md
2cfdc62
@darkgrue
Update README.md
7e1e544
@darkgrue
New IP address logic
dcf35d3 
Cleaned up formatting.
Added additional logic to find default interface to query for IP address to display.
@darkgrue
Fixed crash due to open filehandles.
5459e46 
Fixed open filehandle
Formatting cleanup
Added /contrib directory, support scripts
@darkgrue
Rename functions
d2d9818
@darkgrue
Rename functions
cc69b2b
@darkgrue
Rename functions
dfb3122
@darkgrue
Stylistic cleanup
e6b0bd8
@darkgrue
Major functional changes
ef29c51 
Style changes
Reordered functions by order of call
Fixed types
Fixed -Wall compiler warnings
More robust error handling
Fixed the way top was being called
Added /proc/stat CPU calculation
Added df disk free space calculation
Refactored the way numbers were being pushed to the display that should result in better formatting
Correctly display CPU temperature units of measurement (F displayed instead of C)
@darkgrue
Update README.md
7b2d0f4
@darkgrue
Update README.md
f675ed3
@darkgrue
Update README.md
4796bf9
@darkgrue darkgrue changed the title Major Uppdate Major Update May 9, 2021
This was referenced May 18, 2021
Open
Open
piwi3910 and others added 3 commits June 19, 2021 16:48
@piwi3910
fix typo's
a80b198
@darkgrue
Merge pull request #1 from piwi3910/master
9e1f03b 
fix typo's
@darkgrue
Bugfixes
f7f0001 
Changed alignment of IP address to fix max length address (thanks Piwi for pointing out the problem!)
Added another display digit to CPU load, since there was room.
Added raw background image templates to /contrib directory.
@katanacrimson
Copy link

katanacrimson commented Jul 2, 2021 •
edited
Loading

Liking the result of this MR! Built it and am running it on my own Pis now and it's good stuff.

Suggest adding a mention of introducing the dependency on the wiringpi package into the README, though for raspios users; that package doesn't seem to be a standard install.

edit: wtf, why does my clone's readme not show the dependencies section added. nvm me!

katanacrimson
katanacrimson reviewed Jul 2, 2021
View reviewed changes
README.md
sudo cp ./contrib/U6143_ssd1306.service /etc/systemd/system/
sudo systemctl daemon-reload
sudo systemctl enable U6143_ssd1306.service
sudo systemctl start U6143_ssd1306.service
Copy link

Choose a reason for hiding this comment

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

This could be made one line shorter with sudo systemctl enable --now U6143_ssd1306.service.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, and indeed there's a more efficient way to do that, but going to leave that as a "won't fix" - there's more than one way to skin a cat, and the intent is to show the most common/familiar way to do things, not necessarily the shortest.

katanacrimson
katanacrimson reviewed Jul 2, 2021
View reviewed changes
README.md

## I2C
Begin by enabling the I2C interface:

```bash
sudo raspi-config
Copy link

Choose a reason for hiding this comment

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

Maybe worth mentioning that we can enable it without the ncurses interface?

via: raspi-config nonint do_i2c 0

Copy link
Author

Choose a reason for hiding this comment

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

Same comment as above, it's another way, but it's not really within the scope of this to show all the ways to perform the administrative tasks... Also, I don't think the nonint commands are nominally documented - they're only known through a reading of the source code. But it's definitely an option for advanced administrators to go about the task.

@darkgrue
Remove WiringPi dependency
95da178 
removed dependency
reformatted comments
updated make_tarball.sh
@darkgrue
Update Makefile
6857edc
@darkgrue
Copy link
Author

darkgrue commented Jul 2, 2021

Suggest adding a mention of introducing the dependency on the wiringpi package into the README, though for raspios users; that package doesn't seem to be a standard install.

The wiringpi dependency isn't introduced by my PR, it was a original dependency of the upstream. They've updated it since to remove it (but haven't incorporated any of the PRs). I've refitted that change, as it really simplifies the whole thing and resolves a number of lingering maintainability issues that are created by including it.

@darkgrue darkgrue closed this Jul 2, 2021
@darkgrue darkgrue reopened this Jul 2, 2021
@scyto
Copy link

scyto commented Nov 1, 2021

love this PR, can't understand why it hasn't been accepted by the maintainer

is there a way to get it to show host name in addition to IP?

@darkgrue
Copy link
Author

darkgrue commented Nov 1, 2021

love this PR, can't understand why it hasn't been accepted by the maintainer

Thanks! I wanted to get it working in my environment, and figured I'd share it back.

is there a way to get it to show host name in addition to IP?

Yes, though I don't intend to do so myself. The display code is all bitmapped, so you should be aware there's no fancy scrolling/marquee functions, and getting everything to fit and look good enough is more art than anything.

The labels and such are part of the background bitmap, so those may have to be refactored, depending on how you choose to implement it. I used an online tool to convert images to bitmap arrays to made the adjustments that were needed.

The display code isn't very complicated though, and it's pretty easy to understand, even for a C novice.

@johnbiggs
Copy link

I have a working host name solution that looks fine and works great and is a simple logical change. I'm not a C expert though. Some minor changes might be needed to ensure best C practices.

@johnbiggs
Copy link

johnbiggs commented Nov 22, 2021 •
edited
Loading

Ok. I can't find a way to modify the PR. So add -

char *GetHostname(void)
{
  FILE *fd;
  char buffer[16] = {0};
  char *host = malloc(16);

  fd = fopen("/etc/hostname", "r");
  if(fd == NULL)
  {
    fprintf(stderr, "ssd1306_i2c: Unable to open /etc/hostname.\n");
    return 0;
  }

  fgets(buffer, sizeof(buffer), fd);

  fclose(fd);
  strcpy(host, buffer);

  return host; // Host name
}

AND Modify this routine.

void LCD_DisplaySdMemoryDf(void)
{
  char *mnt_dir = "/";
  FILE *fp;
  char buffer[32] = {0};
  float usedsize = 0;
  float totalsize = 0;
  char used[4] = {0};
  char total[4] = {0};
  char host[16] = {0};

  strcpy(host, GetHostname());                   // Gets the hostname

  fp = popen("df / | awk '{if (NR==2) {print $3 / 1048576\" \"$2 / 1048576}}'", "r");
  if (fp == NULL)
  {
    fprintf(stderr, "ssd1306_i2c: Unable to stat %s filesystem.\n", mnt_dir);
    return;
  }

  fgets(buffer, sizeof(buffer), fp);

  pclose(fp);

  // Parse buffer
  sscanf(buffer, "%f %f", &usedsize, &totalsize);
  sprintf_fix(total, 3, totalsize);
  sprintf_fix(used, 3, usedsize);

/*
  fprintf(stderr, "Disk Used: %f\tTotal: %f\n", usedsize, totalsize);
  fprintf(stderr, "Disk Used: %s GiB\tTotal: %s GiB\n", used, total);
*/
  OLED_ClearLint(0, 1);  //clear IP address
  OLED_ClearLint(2, 4);
  OLED_ShowString(8, 0, host, 8);        // Display Hostname
  OLED_DrawPartBMP(0, 2, 128, 4, BMP, 2);

  OLED_ShowString(90, 3, total, 8);
  OLED_ShowString(55, 3, used, 8);
}

@MarkusNiGit
Copy link

GitHub newbie here. How do I get the new code for this PR? I can see the comparison between old and new but cannot find a way to get the new code only. Want to try out the improvements.

@darkgrue
Copy link
Author

GitHub newbie here. How do I get the new code for this PR? I can see the comparison between old and new but cannot find a way to get the new code only. Want to try out the improvements.

Probably easiest just to pull my fork of the repo. They don't seem to be responding to PRs, and worse, seen to be copying changes from the submitted PRs without attribution (which is pretty reprehensible).

@MarkusNiGit
Copy link

MarkusNiGit commented Jan 26, 2022 •
edited
Loading

So, I would install from
git clone https://github.com/darkgrue/U6143_ssd1306.git
and not from
git clone https://github.com/UCTRONICS/U6143_ssd1306.git

as your README says, correct?

@darkgrue
Copy link
Author

darkgrue commented Jan 26, 2022 via email

@MarkusNiGit
Copy link

MarkusNiGit commented Jan 26, 2022 via email •
edited
Loading

@darkgrue
Copy link
Author

darkgrue commented Jan 26, 2022 via email

@darkgrue
Clear screen on exit.
1906c31 
Catch SIGHUP, SIGINT, and SIGTERM and clear screen (to prevent screen burn-in).
Standardize source file format.
@darkgrue
Fix typos.
8e7d755
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@katanacrimson katanacrimson katanacrimson left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@darkgrue @katanacrimson @scyto @johnbiggs @MarkusNiGit @piwi3910