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

Partial configuration as parameters #3

Draft
wants to merge 46 commits into
base: master
Choose a base branch
from

Conversation

juanjoSanz
Copy link

  • Expose serial device into container
  • Configurable parameters to set serial interface

nx584/CHANGELOG.md Show resolved Hide resolved
nx584/Dockerfile Show resolved Hide resolved
nx584/config.ini Show resolved Hide resolved
@@ -11,3 +14,9 @@ arch:
- i386
ports:
5007/tcp: 5007
options:
serial_dev: "/dev/serial/by-id/usb-Prolific_Technology_Inc._USB-Serial_Controller-if00-port0"
Copy link
Owner

Choose a reason for hiding this comment

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

This is hard-coded to your own serial device. Ideally it should be something more generic?

Copy link
Author

Choose a reason for hiding this comment

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

yes, that's my config, the idea is that this parameter can be easily set in addon config tab

nx584/run.sh Outdated Show resolved Hide resolved
Copy link
Owner

@ivmarkov ivmarkov left a comment

Choose a reason for hiding this comment

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

Overall looks good! A few small remaining changes/suggestions.

But most important - is this ready now? I'm unsure, as you keep pushing more and more stuff. If you plan to continue developing it before I merge, can you mark the PR as "Draft" so that at least I'm aware that this is still work in progress?

baud: "9600"
socat:
enabled: false
host: "192.168.30.1"
Copy link
Owner

Choose a reason for hiding this comment

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

Also here - let's change it to "192.168.0.1" which is kinda like the most popular network?

euro_date_format: true
use_binary_protocol: false
idle_time_heartbeat_seconds: "120"
zones: "NX Main entrance,NX Kitchen,NX Despacho,NX Garage"
Copy link
Owner

Choose a reason for hiding this comment

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

Also here: "Area 1, Area 2, ..."?

Copy link
Author

Choose a reason for hiding this comment

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

I plan to do that, but it is still on development...

@juanjoSanz juanjoSanz marked this pull request as draft October 27, 2023 13:03
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.

2 participants