zeidlos / hamclock-docker

Docker Implementation of Hamclock
MIT License
19 stars 4 forks source link

Issues setting values via EEPROM/hceeprom.pl #6

Closed ChrisRomp closed 9 months ago

ChrisRomp commented 10 months ago

Thanks for the work you've put into this @zeidlos. How are you feeling about maintaining it? I see some open issues and a PR, with no recent activity on the code. I'd be happy to take on maintenance if you want some help, either here or by splitting the project.

That said, in addition to the problems outlined in #2 and addressed with #5, I'm having an issue with the callsign being set via the run.sh script injecting EEPROM values. Even attaching a shell to the container and running it doesn't seem to make the call sign show up correctly. I've even been talking with the HamClock author, Elwood Downey WB0OEW, about it but this got me thinking, why are we doing this way at all? It's a bit of a hack (Elwood even admits as much in the user contrib for his script) and breaks from some standard Docker conventions.

What I've done in my instance is thrown out the config and calling of hceeprom.pl altogether in my version, and instead I'm just mounting a volume to /root/.hamclock where it happily persists its configuration. On first run you'll want to trigger a /restart and go through the setup process, just like any other instance of HamClock, but I think in the end this might be less prone to quirks and therefore easier to manage.

Thoughts?

ChrisRomp commented 10 months ago

Here's my docker-compose.yaml:

version: "3.7"
services:
  web:
    build: .
    ports:
      - "8081:8081"
      - "8080:8080"
    volumes:
      - hamclock-data:/root/.hamclock
    restart: unless-stopped

volumes:
  hamclock-data:

I'll probably add a health check in there for good measure.

ChrisRomp commented 10 months ago

And run.sh can be simplified to:

#!/bin/sh
/usr/local/bin/hamclock -t 20

If I need to go into setup I just call hamclock:8080/restart then click into setup from the web view hamclock:8081/live.html.

ChrisRomp commented 10 months ago

FYI I've built my own version of this which doesn't attempt to manage the EEPROM directly: https://github.com/ChrisRomp/hamclock-docker

zeidlos commented 10 months ago

Hey @ChrisRomp, thank you for reaching out. I've been rather busy with other projects like life, work and family. Since I'm not on github on a daily base and did turn down notifications, I didn't realize there were people interacting with this project. : )

The current state of this is a 'works for me', and that's not satisfactory if other people want to use this. One of the main ideas was to keep this as simple to use as possible, and I found that solution to work okayish, but didn't put more work/effort into it due to the lack of feedback (or so I thought).

Youre welcome to help to contribute through mergerequests, and I promise to be much quicker responding than before (now that I know people are using this, or trying to do so).

On a technical level, I'll review that mergerequest during the weekend. I've already glanced over it and will most likley merge it completly. Also the idea of not using the eeprom calls looks promissing.

ChrisRomp commented 10 months ago

Totally life getting in the way. I'm in GH daily for my job, anyway. I think there's room in the world for our projects to coexist. :)

zeidlos commented 10 months ago

Well, ideally theres no need for several forks being maintained independently. However, I've had a vision in mind and would like the project to adhere to it. Let's have a chat somewhere and see if were on the same page, to maintain this together. I didn't want to shut you out, however I don't know you. I hope you understand the hesitation.

On the other hand, the MIT license was on purpose, so you're free to do whatever. :)

zeidlos commented 10 months ago

Now that I had some time to process what you were writing: My idea with the config file was explicitly to allow setting hamclock up via a config file rather than by hand trough the interface, so you can host multiple instances and wont have to hit the config timeout during setup.

However, mounting the config inside the container to a volume, will persist any changes made within the instance, which would be handy.

Did you figure out why your callsign didn't get set?

ChrisRomp commented 10 months ago

I understand what you're saying. Had we started this conversation before yesterday when I put all this work into it, I probably wouldn't have built my own version. But now that I have, I'm rather attached to it. 😅

As for the MIT license, yes, true, and I use the same license for the same reasons. However, it's important to note that I did not copy your work; I built from scratch following the steps Elwood outlined on the HamClock website.

I never did figure out why my call sign wasn't getting set via the Perl script writing to eeprom. Everything looked correct. I was even emailing with Elwood trying to figure it out. But that's when I took a step back and realized I'd rather persist the data in a volume, anyway. I understand the approach you took, and I like it, if writing to eeprom wasn't a bit of a hack by Elwood's own admission.

You can still host multiple instances this way, just having the volumes in different places.

I'm also building this precompiled to make it easier for users (just copy the docker-compose.yaml and be done). This will also make it easier for NAS users that can run Docker, e.g., QNAP and Synology. I'm currently working on a build that will run on ARM/v7 (Raspberry Pi) running Docker since the make/gcc is returning a 1 on a warning, it's failing. I just need to make sure this doesn't end up costing me a lot in storage. 😬

zeidlos commented 10 months ago

I wouldn't care if you took any of my code or not. It's open source for a reason. Did Elwood give you permission to redistribute his runtime? I didn't ask but found his website forbidding any rehosting, which a precompiled docker container clearly would be. That's why I didn't do it.

zeidlos commented 10 months ago

By the way, setting the callsign via eeprom doesn't work for me now either, so I guess it's broken with a newer release of hamclock.

ChrisRomp commented 10 months ago

RE: Redistribution, I've been emailing with him on the build issues for ARM/v7, but I think it'll be good to call that out with him. Thanks for asking.

ChrisRomp commented 10 months ago

He said he's ok with it as long as the updater works within the container.