h3n3/kiauh
1
0
Fork 0
Code Issues 46 Pull Requests 10 Packages Projects Releases Wiki Activity

feat: add Obico for Klipper to KIAUH #227

Merged
kennethjiang merged 36 commits from moonraker_obico into master 2022-08-15 19:44:04 +02:00
Conversation 10 Commits 36 Files Changed 11 +2 -1
1 changed files with 2 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 78cc8bbffb - Show all commits

3
scripts/obico.sh
View File

@@ -201,10 +201,11 @@ function moonraker_obico_setup_dialog() {
dw-0 commented 2022-08-13 11:51:56 +02:00 (Migrated from github.com)
Review
Copy Link
local log="${KLIPPER_LOGS}"

This variable seems to be unused?

```ssh local log="${KLIPPER_LOGS}" ``` This variable seems to be unused?
dw-0 commented 2022-08-13 11:51:56 +02:00 (Migrated from github.com)
Review
Copy Link
local log="${KLIPPER_LOGS}"

This variable seems to be unused?

```ssh local log="${KLIPPER_LOGS}" ``` This variable seems to be unused?
kennethjiang commented 2022-08-13 16:37:53 +02:00 (Migrated from github.com)
Review
Copy Link

Another good catch. I guess I planed to use it but ended up using "${KLIPPER_LOGS}" directly. Feel free to remove it.

Another good catch. I guess I planed to use it but ended up using `"${KLIPPER_LOGS}"` directly. Feel free to remove it.
kennethjiang commented 2022-08-13 16:37:53 +02:00 (Migrated from github.com)
Review
Copy Link

Another good catch. I guess I planed to use it but ended up using "${KLIPPER_LOGS}" directly. Feel free to remove it.

Another good catch. I guess I planed to use it but ended up using `"${KLIPPER_LOGS}"` directly. Feel free to remove it.
dw-0 commented 2022-08-14 15:40:07 +02:00 (Migrated from github.com)
Review
Copy Link

Can this variable be local? Or is it required to be global? I see it only being used inside the moonraker_obico_needs_linking function

Can this variable be local? Or is it required to be global? I see it only being used inside the `moonraker_obico_needs_linking` function
dw-0 commented 2022-08-14 15:40:07 +02:00 (Migrated from github.com)
Review
Copy Link

Can this variable be local? Or is it required to be global? I see it only being used inside the moonraker_obico_needs_linking function

Can this variable be local? Or is it required to be global? I see it only being used inside the `moonraker_obico_needs_linking` function
dw-0 commented 2022-08-14 15:41:04 +02:00 (Migrated from github.com)
Review
Copy Link

This variable seems to be unused now?

This variable seems to be unused now?
dw-0 commented 2022-08-14 15:41:04 +02:00 (Migrated from github.com)
Review
Copy Link

This variable seems to be unused now?

This variable seems to be unused now?
dw-0 commented 2022-08-14 15:44:14 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
dw-0 commented 2022-08-14 15:44:14 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
dw-0 commented 2022-08-14 15:44:24 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
dw-0 commented 2022-08-14 15:44:24 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
fi
dw-0 commented 2022-08-14 15:43:41 +02:00 (Migrated from github.com)
Review
Copy Link

The moonraker_obico_config function defined in line 24 does not take in any arguments. So the 0 you pass there (and a few lines below the ${i}) won't get used inside the function. Im not quite sure what the intention was, if the arguments are obsolete or if the moonraker_obico_config function is incomplete?

The `moonraker_obico_config` function defined in line 24 does not take in any arguments. So the `0` you pass there (and a few lines below the `${i}`) won't get used inside the function. Im not quite sure what the intention was, if the arguments are obsolete or if the `moonraker_obico_config` function is incomplete?
kennethjiang commented 2022-08-14 16:08:07 +02:00 (Migrated from github.com)
Review
Copy Link

This is a really good catch! The function moonraker_obico_config is supposed to use ${1}, not ${i}. Not sure how my testing passed... Fixing it now.

This is a really good catch! The function `moonraker_obico_config` is supposed to use `${1}`, not `${i}`. Not sure how my testing passed... Fixing it now.
kennethjiang commented 2022-08-14 16:16:53 +02:00 (Migrated from github.com)
Review
Copy Link

Should be fixed in 276c123. moonraker_obico_config should take an argument, which is the index of a specific instance in multi_instance_names

Should be fixed in [276c123](https://github.com/th33xitus/kiauh/pull/227/commits/276c123d3d6d0d3609e0f4b179b545d7d53f40cd). `moonraker_obico_config` should take an argument, which is the index of a specific instance in `multi_instance_names`
### Step 7: Link to the Obico server if necessary
local instance_name
dw-0 commented 2022-08-13 11:51:56 +02:00 (Migrated from github.com)
Review
Copy Link
local log="${KLIPPER_LOGS}"

This variable seems to be unused?

```ssh local log="${KLIPPER_LOGS}" ``` This variable seems to be unused?
kennethjiang commented 2022-08-13 16:37:53 +02:00 (Migrated from github.com)
Review
Copy Link

Another good catch. I guess I planed to use it but ended up using "${KLIPPER_LOGS}" directly. Feel free to remove it.

Another good catch. I guess I planed to use it but ended up using `"${KLIPPER_LOGS}"` directly. Feel free to remove it.
dw-0 commented 2022-08-14 15:40:07 +02:00 (Migrated from github.com)
Review
Copy Link

Can this variable be local? Or is it required to be global? I see it only being used inside the moonraker_obico_needs_linking function

Can this variable be local? Or is it required to be global? I see it only being used inside the `moonraker_obico_needs_linking` function
dw-0 commented 2022-08-14 15:41:04 +02:00 (Migrated from github.com)
Review
Copy Link

This variable seems to be unused now?

This variable seems to be unused now?
dw-0 commented 2022-08-14 15:44:14 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
dw-0 commented 2022-08-14 15:44:24 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
local not_linked_instances=()
# Refetch systemd service again since additional services may have been newly installed
for service in $(moonraker_obico_systemd); do
local instance_name="$(get_instance_name "${service}" moonraker-obico)"
dw-0 commented 2022-08-13 11:51:56 +02:00 (Migrated from github.com)
Review
Copy Link
local log="${KLIPPER_LOGS}"

This variable seems to be unused?

```ssh local log="${KLIPPER_LOGS}" ``` This variable seems to be unused?
kennethjiang commented 2022-08-13 16:37:53 +02:00 (Migrated from github.com)
Review
Copy Link

Another good catch. I guess I planed to use it but ended up using "${KLIPPER_LOGS}" directly. Feel free to remove it.

Another good catch. I guess I planed to use it but ended up using `"${KLIPPER_LOGS}"` directly. Feel free to remove it.
dw-0 commented 2022-08-14 15:40:07 +02:00 (Migrated from github.com)
Review
Copy Link

Can this variable be local? Or is it required to be global? I see it only being used inside the moonraker_obico_needs_linking function

Can this variable be local? Or is it required to be global? I see it only being used inside the `moonraker_obico_needs_linking` function
dw-0 commented 2022-08-14 15:41:04 +02:00 (Migrated from github.com)
Review
Copy Link

This variable seems to be unused now?

This variable seems to be unused now?
dw-0 commented 2022-08-14 15:44:14 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
dw-0 commented 2022-08-14 15:44:24 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
instance_name="$(get_instance_name "${service}" moonraker-obico)"
dw-0 commented 2022-08-13 11:51:56 +02:00 (Migrated from github.com)
Review
Copy Link
local log="${KLIPPER_LOGS}"

This variable seems to be unused?

```ssh local log="${KLIPPER_LOGS}" ``` This variable seems to be unused?
kennethjiang commented 2022-08-13 16:37:53 +02:00 (Migrated from github.com)
Review
Copy Link

Another good catch. I guess I planed to use it but ended up using "${KLIPPER_LOGS}" directly. Feel free to remove it.

Another good catch. I guess I planed to use it but ended up using `"${KLIPPER_LOGS}"` directly. Feel free to remove it.
dw-0 commented 2022-08-14 15:40:07 +02:00 (Migrated from github.com)
Review
Copy Link

Can this variable be local? Or is it required to be global? I see it only being used inside the moonraker_obico_needs_linking function

Can this variable be local? Or is it required to be global? I see it only being used inside the `moonraker_obico_needs_linking` function
dw-0 commented 2022-08-14 15:41:04 +02:00 (Migrated from github.com)
Review
Copy Link

This variable seems to be unused now?

This variable seems to be unused now?
dw-0 commented 2022-08-14 15:44:14 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
dw-0 commented 2022-08-14 15:44:24 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
if ! is_moonraker_obico_linked "${instance_name}"; then
not_linked_instances+=( "${instance_name}" )
fi
dw-0 commented 2022-08-13 11:51:56 +02:00 (Migrated from github.com)
Review
Copy Link
local log="${KLIPPER_LOGS}"

This variable seems to be unused?

```ssh local log="${KLIPPER_LOGS}" ``` This variable seems to be unused?
dw-0 commented 2022-08-13 11:51:56 +02:00 (Migrated from github.com)
Review
Copy Link
local log="${KLIPPER_LOGS}"

This variable seems to be unused?

```ssh local log="${KLIPPER_LOGS}" ``` This variable seems to be unused?
kennethjiang commented 2022-08-13 16:37:53 +02:00 (Migrated from github.com)
Review
Copy Link

Another good catch. I guess I planed to use it but ended up using "${KLIPPER_LOGS}" directly. Feel free to remove it.

Another good catch. I guess I planed to use it but ended up using `"${KLIPPER_LOGS}"` directly. Feel free to remove it.
kennethjiang commented 2022-08-13 16:37:53 +02:00 (Migrated from github.com)
Review
Copy Link

Another good catch. I guess I planed to use it but ended up using "${KLIPPER_LOGS}" directly. Feel free to remove it.

Another good catch. I guess I planed to use it but ended up using `"${KLIPPER_LOGS}"` directly. Feel free to remove it.
dw-0 commented 2022-08-14 15:40:07 +02:00 (Migrated from github.com)
Review
Copy Link

Can this variable be local? Or is it required to be global? I see it only being used inside the moonraker_obico_needs_linking function

Can this variable be local? Or is it required to be global? I see it only being used inside the `moonraker_obico_needs_linking` function
dw-0 commented 2022-08-14 15:40:07 +02:00 (Migrated from github.com)
Review
Copy Link

Can this variable be local? Or is it required to be global? I see it only being used inside the moonraker_obico_needs_linking function

Can this variable be local? Or is it required to be global? I see it only being used inside the `moonraker_obico_needs_linking` function
dw-0 commented 2022-08-14 15:41:04 +02:00 (Migrated from github.com)
Review
Copy Link

This variable seems to be unused now?

This variable seems to be unused now?
dw-0 commented 2022-08-14 15:41:04 +02:00 (Migrated from github.com)
Review
Copy Link

This variable seems to be unused now?

This variable seems to be unused now?
dw-0 commented 2022-08-14 15:44:14 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
dw-0 commented 2022-08-14 15:44:14 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
dw-0 commented 2022-08-14 15:44:24 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)
dw-0 commented 2022-08-14 15:44:24 +02:00 (Migrated from github.com)
Review
Copy Link

See comment above :)

See comment above :)