Makes firmware uploads to the cloud explicit. #79

gckzl · 2023-07-16T07:01:45Z

stops making autodumps when run without arguments.
adds a separate upload command, to make it clear the data is leaving the local network
moves the backup / upload work to the main process: no more background processes
refactors the MTD code to work with cJSON objects instead of printing to stdout

One side-effect of this change is that STDOUT isn't flushed anymore before trying to detect the sensors: all output happens after all the data has been collected.

Another change is that the output skips the initial "---" marker, making it a "bare document" in YAML terms. Since we're only producing a single document, this shouldn't make any difference.

* stops making autodumps when run without arguments. * adds a separate `upload` command, to make it clear the data is leaving the local network * moves the backup / upload work to the main process: no more background processes * refactors the MTD code to work with cJSON objects instead of printing to stdout One side-effect of this change is that STDOUT isn't flushed anymore before trying to detect the sensors: all output happens after all the data has been collected. Another change is that the output skips the initial "---" marker, making it a "bare document" in YAML terms. Since we're only producing a single document, this shouldn't make any difference.

gckzl · 2023-07-16T07:04:44Z

@widgetii, I've checked that the code compiles, but I didn't get to test it on a device yet (my camera is currently bricked, I'm working on reflashing it, but it's gonna take a while, as it involves soldering). So, if the patch looks good to you, I'd be grateful if you could see if it works on a real device. Otherwise, I'll be able to test it myself in mid-August.

Given the following structure (presented here as JSON): ┌────────────────────────────────────────────────────────┐ │ { │ │ "rom": { │ │ "attr": "val", │ │ "other": 1.6 │ │ }, │ │ "ram": { │ │ "size": 4096, │ │ "name": "mRA\bM" │ │ }, │ │ "list" : [ │ │ "item1", │ │ "item2", │ │ [ │ │ "sub1", │ │ "sub2" │ │ ], │ │ "item3", │ │ [ │ │ [ "subsub1" ], │ │ [ 1, 2, { "val": 3, "spelled": "three" }, 4 ], │ │ [ "subsub3" ] │ │ ], │ │ "item4" │ │ ] │ │ } │ └────────────────────────────────────────────────────────┘ The old code produced: ┌────────────────────────────────────────────────────────┐ │ rom: │ │ attr: val │ │ other: 1.6 │ │ ram: │ │ size: 4096 │ │ name: "mRA\bM" │ │ list: │ │ - item1 │ │ - item2 │ │ - │ │ - sub1 │ │ - sub2 │ │ - item3 │ │ - │ │ - │ │ - subsub1 │ │ - │ │ - 1 │ │ - 2 │ │ - val: 3 │ │ spelled: three │ │ - 4 │ │ - │ │ - subsub3 │ │ - item4 │ └────────────────────────────────────────────────────────┘ The new code now gives: ┌────────────────────────────────────────────────────────┐ │--- │ │rom: │ │ attr: val │ │ other: 1.6 │ │ram: │ │ size: 4096 │ │ name: "mRA\bM" │ │list: │ │ - item1 │ │ - item2 │ │ - - sub1 │ │ - sub2 │ │ - item3 │ │ - - - subsub1 │ │ - - 1 │ │ - 2 │ │ - val: 3 │ │ spelled: three │ │ - 4 │ │ - - subsub3 │ │ - item4 │ └────────────────────────────────────────────────────────┘ Also added a basic test.

gckzl · 2023-07-17T22:50:21Z

The wrong YAML output turned out to be a bug in the cYAML implementation, and it fundamentally couldn't handle nested lists correctly. I ended up rewriting the whole thing. Please take another look.

viktorxda · 2023-07-18T12:00:00Z

Small layout difference.

Edit:
Seems like it is correctly done on the PR as it follows the same syntax.

Original:

rom:
  - type: nor
    block: 64K
    partitions:

[...]

sensors:
- vendor: GalaxyCore
  model: GC4653
  control:
    bus: 0
    type: i2c
    addr: 0x52

PR:

rom:
  - type: nor
    block: 64K
    partitions:

[...]

sensors:
  - vendor: GalaxyCore
    model: GC4653
    control:
      bus: 0
      type: i2c
      addr: 0x52

…lumn 0. This seems to have changed in the YAML 1.2.2 spec: https://yaml.org/spec/1.2.2/#21-collections american: - Boston Red Sox - Detroit Tigers - New York Yankees national: - New York Mets - Chicago Cubs - Atlanta Braves Previously: https://yaml.org/spec/1.2.1/#id2759963 american: - Boston Red Sox - Detroit Tigers - New York Yankees national: - New York Mets - Chicago Cubs - Atlanta Braves I find the old style to make much more sense, but whaddarayagonnado?

gckzl · 2023-07-18T12:44:35Z

In this case, we either render the "rom" section as it was, or the "sensors" section. I find the "rom" style to make more sense, but the "sensors" style seems to be specified on the YAML 1.2.2 spec, so I implemented the change. I would gladly revert this last change, though, spec be damned.

gckzl · 2023-07-18T12:54:12Z

Hang on, in the last commit I've introduced a bug when rendering top-level lists. I'll see how I can fix it, but really, we should stick to the style in YAML 1.2.1 and earlier, and just drop that commit.

@widgetii, @viktorxda, what do you think?

viktorxda · 2023-07-18T13:10:01Z

Personally, as long as the syntax is conclusive I see no problem with both versions, but that needs to be decided by the maintainers. I can primarily give feedback or test the recent changes.

Here is the output of the different versions:

Original

---
chip:
  vendor: SigmaStar
  model: SSC333X/SSC335X/SSC337X
  id: 999999999999
  tag: MVX1##I6gf74c9b5CMN_ROM######XVM
board:
  vendor: OpenIPC
  version: 2.3.07.15
ethernet:
  mac: "99:99:99:99:99:99"
rom:
  - type: nor
    block: 64K
    partitions:
      - name: boot
        size: 0x40000
        sha1: 1c85bdf3
      - name: env
        size: 0x10000
        sha1: cf91c01c
        contains:
          - name: uboot-env
            offset: 0x0
      - name: kernel
        size: 0x300000
        sha1: 374fbbd5
      - name: rootfs
        size: 0xa00000
        sha1: c8dce8cc
      - name: rootfs_data
        size: 0x2b0000
        path: /overlay,jffs2,rw
    size: 16M
ram:
  total: 128M
  media: 56M
firmware:
  u-boot: "2015.01 (Jul 01 2023 - 00:01:29)"
  kernel: "4.9.84 (PREEMPT Sat Jul 15 10:52:48 UTC 2023)"
  toolchain: buildroot-gcc-12.2.0
  main-app: /usr/bin/majestic
sensors:
- vendor: GalaxyCore
  model: GC4653
  control:
    bus: 0
    type: i2c
    addr: 0x52

PR YAML 1.2.1

---
chip:
  vendor: SigmaStar
  model: SSC333X/SSC335X/SSC337X
  id: 999999999999
  tag: MVX1##I6gf74c9b5CMN_ROM######XVM
board:
  vendor: OpenIPC
  version: 2.3.07.15
ethernet:
  mac: "99:99:99:99:99:99"
rom:
  - type: nor
    block: 64K
    partitions:
      - name: boot
        size: 0x40000
        sha1: 1c85bdf3
      - name: env
        size: 0x10000
        sha1: cf91c01c
        contains:
          - name: uboot-env
            offset: 0x0
      - name: kernel
        size: 0x300000
        sha1: 374fbbd5
      - name: rootfs
        size: 0xa00000
        sha1: c8dce8cc
      - name: rootfs_data
        size: 0x2b0000
        path: /overlay,jffs2,rw
    size: 16M
ram:
  total: 128M
  media: 56M
firmware:
  u-boot: "2015.01 (Jul 01 2023 - 00:01:29)"
  kernel: "4.9.84 (PREEMPT Sat Jul 15 10:52:48 UTC 2023)"
  toolchain: buildroot-gcc-12.2.0
  main-app: /usr/bin/majestic
sensors:
  - vendor: GalaxyCore
    model: GC4653
    control:
      bus: 0
      type: i2c
      addr: 0x52

PR YAML 1.2.2

---
chip:
  vendor: SigmaStar
  model: SSC333X/SSC335X/SSC337X
  id: 999999999999
  tag: MVX1##I6gf74c9b5CMN_ROM######XVM
board:
  vendor: OpenIPC
  version: 2.3.07.15
ethernet:
  mac: "99:99:99:99:99:99"
rom:
- type: nor
  block: 64K
  partitions:
    - name: boot
      size: 0x40000
      sha1: 1c85bdf3
    - name: env
      size: 0x10000
      sha1: cf91c01c
      contains:
        - name: uboot-env
          offset: 0x0
    - name: kernel
      size: 0x300000
      sha1: 374fbbd5
    - name: rootfs
      size: 0xa00000
      sha1: c8dce8cc
    - name: rootfs_data
      size: 0x2b0000
      path: /overlay,jffs2,rw
  size: 16M
ram:
  total: 128M
  media: 56M
firmware:
  u-boot: "2015.01 (Jul 01 2023 - 00:01:29)"
  kernel: "4.9.84 (PREEMPT Sat Jul 15 10:52:48 UTC 2023)"
  toolchain: buildroot-gcc-12.2.0
  main-app: /usr/bin/majestic
sensors:
- vendor: GalaxyCore
  model: GC4653
  control:
    bus: 0
    type: i2c
    addr: 0x52

This was broken by the hack introduced in the previous commit.

gckzl · 2023-07-18T13:20:09Z

I've also fixed the handling of top-level lists, but I must say, my favorite is the "PR YAML 1.2.1", so I would drop these last two commits (would probably keep the tests).

widgetii · 2023-07-20T18:34:20Z

Thank you, looks very good, I'll test everything on my devices this weekend

gckzl · 2023-07-25T17:35:32Z

Hi @widgetii, did you get a chance to play with this PR? Also, any preference among the options presented in #79 (comment)?

widgetii · 2023-07-25T17:54:07Z

Not yet, may be this weekend. If colleagues can make tests on their real devices, let's merge it early

viktorxda · 2023-07-25T19:51:51Z

Here is a output test from a ingenic device:

ipctool-mipsel

---
chip:
  vendor: Ingenic
  model: T21N
board:
  vendor: OpenIPC
  version: 2.3.07.23
ethernet:
  mac: "xx:xx:xx:xx:xx:xx"
rom:
- type: nor
  block: 32K
  partitions:
    - name: boot
      size: 0x40000
      sha1: 35bc366c
    - name: env
      size: 0x10000
      sha1: b38f0062
      contains:
        - name: uboot-env
          offset: 0x0
    - name: kernel
      size: 0x300000
      sha1: 13ebde61
    - name: rootfs
      size: 0xa00000
      path: /,squashfs
      sha1: 5af257af
    - name: rootfs_data
      size: 0x2b0000
      path: /overlay,jffs2,rw
  size: 16M
ram:
  total: 64M
  media: 24M
firmware:
  kernel: "3.10.14__isvp_turkey_1.0__ (PREEMPT Sun Jul 23 18:06:21 UTC 2023)"
  toolchain: buildroot-gcc-12.2.0
sensors:
- vendor: Silicon Optronics
  model: JXF37
  control:
    bus: 0
    type: i2c
    addr: 0x80

One thing to note is the slight delay between the execution and the output, as previously mentioned.
Maybe you could add a message that indicates that the process is collecting the information.

widgetii · 2023-07-26T04:17:37Z

Maybe you could add a message that indicates that the process is collecting the information.

In this case it will be not valid YAML anymore

viktorxda · 2023-07-26T05:02:37Z

I see. Otherwise I didn't notice any problems, here is a test sample from another device:

openipc-hi3516ev300

root@openipc-hi3516ev300:~# /tmp/ipctool
---
chip:
  vendor: HiSilicon
  model: 3516EV300
  id: 000000000000000000000000000000000000000000000000
board:
  vendor: OpenIPC
  version: 2.3.07.23
  possible-IR-cut-GPIO: 7
ethernet:
  mac: "xx:xx:xx:xx:xx:xx"
  u-mdio-phyaddr: 1
  phy-id: 0x20669903
  d-mdio-phyaddr: 0
rom:
- type: nor
  block: 64K
  partitions:
    - name: boot
      size: 0x40000
      sha1: c35c806b
    - name: env
      size: 0x10000
      sha1: 017ca834
      contains:
        - name: uboot-env
          offset: 0x0
    - name: kernel
      size: 0x300000
      sha1: fbe4ae1b
    - name: rootfs
      size: 0xa00000
      sha1: ea1e905e
    - name: rootfs_data
      size: 0x2b0000
      path: /overlay,jffs2,rw
  size: 16M
  addr-mode: 3-byte
ram:
  total: 128M
  media: 96M
firmware:
  kernel: "4.9.37 (Sun Jul 23 17:53:37 UTC 2023)"
  toolchain: buildroot-gcc-12.2.0
  sdk: "Hi3516EV200_MPP_V1.0.1.2 B030 Release (Oct 18 2019, 18:21:00)"
  main-app: /usr/bin/majestic
sensors:
- vendor: Sony
  model: IMX335
  control:
    bus: 0
    type: i2c
    addr: 0x34
  data:
    type: MIPI
    input-data-type: DATA_TYPE_RAW_12BIT
    lane-id:
      - 0
      - 1
      - 2
      - 3
    image: 2592x1944
  clock: 37.125MHz

widgetii · 2023-07-26T07:26:47Z

Ok, let's merge then

@gckzl thank for your efforts

gckzl · 2023-07-28T10:42:10Z

@widgetii, @viktorxda Thanks for your help.

While working on the YAML code, I noticed it didn't handle empty strings, empty objects and empty arrays correctly: in that case, it would output invalid YAML (this limitation was present also before my re-write). I'll send a separate PR for this, as I didn't want to keep expanding this PR.

dimerr · 2024-01-18T10:26:43Z

@gckzl

This PR breaks output of https://github.com/OpenIPC/ipctool/blob/master/src/boards/xm.c#L47
Before:


    chip:
      name: "XM_XM25QH128C"
      id: 0x204018

After:
chip: " name: \"XM_XM25QH128C\"\n id: 0x204018\n"

gckzl · 2024-04-29T16:34:50Z

@dimerr, sorry I hadn't realized nor_chip contained YAML instead of a simple string.

I've sent out PR #128 with a fix.

gckzl mentioned this pull request Jul 16, 2023

ipctool collects complete firmware dumps without the user's knowledge or approval #78

Closed

gckzl added 2 commits July 18, 2023 00:16

Modifies examine_part() in mtd.c to use cJSON instead of plain strings.

e9b24ec

Fixes handling of top-level lists.

912ca60

This was broken by the hack introduced in the previous commit.

widgetii merged commit 81c95c3 into OpenIPC:master Jul 26, 2023

gckzl deleted the no-background-uploads branch July 28, 2023 10:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Makes firmware uploads to the cloud explicit. #79

Makes firmware uploads to the cloud explicit. #79

gckzl commented Jul 16, 2023

gckzl commented Jul 16, 2023 •

edited

Loading

gckzl commented Jul 17, 2023

viktorxda commented Jul 18, 2023 •

edited

Loading

gckzl commented Jul 18, 2023 •

edited

Loading

gckzl commented Jul 18, 2023

viktorxda commented Jul 18, 2023

gckzl commented Jul 18, 2023

widgetii commented Jul 20, 2023

gckzl commented Jul 25, 2023

widgetii commented Jul 25, 2023

viktorxda commented Jul 25, 2023

widgetii commented Jul 26, 2023

viktorxda commented Jul 26, 2023

widgetii commented Jul 26, 2023

gckzl commented Jul 28, 2023

dimerr commented Jan 18, 2024

gckzl commented Apr 29, 2024

Makes firmware uploads to the cloud explicit. #79

Makes firmware uploads to the cloud explicit. #79

Conversation

gckzl commented Jul 16, 2023

gckzl commented Jul 16, 2023 • edited Loading

gckzl commented Jul 17, 2023

viktorxda commented Jul 18, 2023 • edited Loading

gckzl commented Jul 18, 2023 • edited Loading

gckzl commented Jul 18, 2023

viktorxda commented Jul 18, 2023

gckzl commented Jul 18, 2023

widgetii commented Jul 20, 2023

gckzl commented Jul 25, 2023

widgetii commented Jul 25, 2023

viktorxda commented Jul 25, 2023

widgetii commented Jul 26, 2023

viktorxda commented Jul 26, 2023

widgetii commented Jul 26, 2023

gckzl commented Jul 28, 2023

dimerr commented Jan 18, 2024

gckzl commented Apr 29, 2024

gckzl commented Jul 16, 2023 •

edited

Loading

viktorxda commented Jul 18, 2023 •

edited

Loading

gckzl commented Jul 18, 2023 •

edited

Loading