diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3c1a807..d52fa35 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,15 +1,34 @@ name: build and release on: - release: - types: [ created, published ] - + # Exactly one build per release, driven by the tag push. + # + # There is deliberately no `release:` trigger. Creating a release on a new + # tag also pushes that tag, so having both fires two runs for one release — + # and `types: [created, published]` fired twice again on its own, because + # publishing a non-draft release emits both events. + # + # Pushing a tag builds every platform and attaches the artifacts to a *draft* + # release. Publishing that draft does not rebuild: nothing here listens for + # it, and the binaries are already attached. + # + # Note `release: created` could not have covered the draft case anyway — + # GitHub does not trigger workflows for the created/edited/deleted activity + # types on draft releases, and a draft has no git tag until it is published. push: tags: - 'v*' + # Manual builds for sharing test binaries; uploaded as workflow artifacts + # rather than to a release. workflow_dispatch: +# One run per tag. Queues rather than cancels: these runs upload release +# assets, so killing one mid-upload would leave the release incomplete. +concurrency: + group: release-${{ github.ref_name }} + cancel-in-progress: false + permissions: contents: write @@ -17,8 +36,23 @@ env: CARGO_TERM_COLOR: always jobs: + # Create the draft release once, before the matrix fans out. Four parallel + # jobs each calling action-gh-release on a tag with no existing release would + # race to create it; with this job they only ever add assets to it. + create-release: + name: create draft release + if: startsWith(github.ref, 'refs/tags/') + runs-on: ubuntu-latest + steps: + - uses: softprops/action-gh-release@v3 + with: + draft: true + build: name: build (${{ matrix.platform.label }}-${{ matrix.platform.arch }}) + needs: [create-release] + # Still build on workflow_dispatch, where create-release is skipped. + if: ${{ !cancelled() && needs.create-release.result != 'failure' }} runs-on: ${{ matrix.platform.runner }} strategy: fail-fast: false @@ -115,7 +149,7 @@ jobs: if: runner.os == 'macOS' shell: bash env: - APP_VERSION: ${{ github.event.release.tag_name || github.ref_name }} + APP_VERSION: ${{ github.ref_name }} run: | set -euo pipefail rm -rf dist && mkdir -p dist @@ -152,7 +186,7 @@ jobs: (Get-FileHash "dist/${{ matrix.platform.archive }}" -Algorithm SHA256).Hash.ToLower() | Set-Content "dist/${{ matrix.platform.archive }}.sha256" - name: Release artifacts (non-mac) - if: github.event_name == 'release' && runner.os != 'macOS' + if: startsWith(github.ref, 'refs/tags/') && runner.os != 'macOS' uses: softprops/action-gh-release@v3 with: files: | @@ -160,7 +194,7 @@ jobs: dist/${{ matrix.platform.archive }}.sha256 - name: Release artifacts (mac) - if: github.event_name == 'release' && runner.os == 'macOS' + if: startsWith(github.ref, 'refs/tags/') && runner.os == 'macOS' uses: softprops/action-gh-release@v3 with: files: | diff --git a/docs/BT_PROTOCOL.md b/docs/BT_PROTOCOL.md index 346beb7..6796666 100644 --- a/docs/BT_PROTOCOL.md +++ b/docs/BT_PROTOCOL.md @@ -45,6 +45,34 @@ a valid checksum, the device responds with error code `0x06` in the ACK. Implementation: `el15_bt::checksum(data: &[u8]) -> u8`. +## Command pacing (required) + +The write characteristic advertises WRITE_WITHOUT_RESPONSE, and `Device::send` +uses it. That has **no flow control**: commands written back-to-back are +silently dropped, with no error reported to the host. + +Leave at least **120 ms** between consecutive commands +(`el15_bt::INTER_COMMAND_GAP`). Use `Device::send_sequence()` for any burst +rather than consecutive `send()` calls. + +Measured on HW:2.0 / SW:1.7 — three commands (set mode, set setpoint, set CAP +current) sent with no gap: + +``` +>> af 07 03 03 01 02 41 set mode CAP +>> af 07 03 04 04 … set setpoint (no gap) +>> af 07 03 09 01 04 39 load on (no gap) +<< df 07 03 03 01 00 13 ACK for the mode command only +<< df 01 04 39 01 02 e0 malformed — contains 04 39 from the outgoing frame +``` + +The CAP current read back **unchanged 3 times out of 3**; spaced 120 ms apart +the identical sequence took effect 3 times out of 3. Symptomatically this looks +like a UI that needs several button presses before a command "takes". + +The poll timer must also be held off for the duration of a burst, otherwise the +periodic poll becomes one more back-to-back write (`AppState::pause_ticks_for`). + ## Connection Handshake After BLE connection and characteristic subscription, the host **must** send @@ -194,14 +222,111 @@ The `el15-bt` library emits these events from the notification stream: | ---------------------------- | ----------------------------------------------------- | | `Status(EL15Status)` | Any 28-byte notification with header `DF 07 03 08` | | `FirmwareVersion(String)` | Init response with header `DF FF FF`; parsed by `parse_firmware_version` | +| `CapCurrent(f32)` | CAP discharge-current reply `DF 07 03 0A 04 `, in Amps | | `RawNotification(Vec)` | Every notification, unfiltered (for debugging) | | `Disconnected` | GATT notification stream ends | +### CAP discharge current (cmd `0x05` write / `0x0A` read) + +The capacity-test discharge current has its **own opcode** — the ordinary +`set setpoint` (`0x04`) does not reach it. + +| Direction | Bytes | +| --------- | ---------------------------------------- | +| Write | `AF 07 03 05 04 ` | +| Read | `AF 07 03 0A 00 3D` | +| Response | `DF 07 03 0A 04 ` | + +Verified end-to-end on HW:2.0 / SW:1.7: + +``` +>> af 07 03 0a 00 3d read +<< df 07 03 0a 04 00 00 a0 40 29 5.0 A (device showed 5000 mA) +>> af 07 03 05 04 00 00 00 40 fe write 2.0 A +<< df 07 03 05 01 00 11 ACK, status 00 +>> af 07 03 0a 00 3d read +<< df 07 03 0a 04 04 00 00 40 c5 2.0 A (device showed 2000 mA) +``` + +Range is `0 – 12000 mA` (manual §3.4.1). The value is in **Amps**, not +milliamps. The device stores milliamps internally, so a value written as `5.0` +reads back as `5.0000010` — never compare the readback for exact equality. + +The current is **not** carried in the status packet: in CAP mode bytes 23..27 +hold the measured capacity. `0x0A` is the only way to observe it. The register +is global — it reads the same in CC, CAP and DCR mode. + +### Command map + +Every opcode in `AF 07 03 …` was probed on real hardware. `len = 0` +means *read*, `len > 0` means *write*. + +| cmd | Read (`len=0`) | Write | Meaning | +| ------ | ----------------------- | ---------------- | -------------------------------- | +| `0x00` | code `03` | code `03` | invalid | +| `0x01` | code `00` | — | accepted, no observed effect | +| `0x02` | 3 bytes `00 11 14` | — | **unidentified** (constant) | +| `0x03` | — | `len=1` mode | set mode | +| `0x04` | — | `len=4` f32 | set setpoint (CC/CV/CR/CP only) | +| `0x05` | code `04` | `len=4` f32 A | **set CAP discharge current** | +| `0x06` | code `04` | **only `len=10`** | **unidentified** (see notes) | +| `0x07` | device name | — | info | +| `0x08` | 28-byte status | — | poll | +| `0x09` | — | `len=1` | load on / off / lock | +| `0x0A` | 4 bytes f32 A | code `04` | **read CAP discharge current** | +| `0x0B`+| code `05` | — | unknown | + +Acknowledgement status byte (`DF 07 03 01 `): + +| Status | Meaning | +| ------ | ---------------------------------------- | +| `0x00` | accepted | +| `0x03` | invalid command | +| `0x04` | command exists, wrong payload length | +| `0x05` | unknown command | + ## Operational notes -- **CAP mode** uses the discharge current stored in device memory (set via - front panel or previous session). The `set setpoint` command does not affect - CAP discharge current. +- **DCR test currents and timer are not available over BLE.** Verified on + HW:2.0 / SW:1.7: with the device at its factory 20 mA / 1000 mA, neither + `0x04` (setpoint) nor `0x05` (CAP current) moves them — both are acknowledged + with status `00` and the status packet keeps reporting 20 mA / 1000 mA. Like + the CAP cutoff these are front-panel settings (manual §3.4.2 "DCR Params"). + They **are** readable: in DCR mode status bytes 15..19 and 19..23 carry I1 and + I2 in Amps, so the app displays them read-only. +- **`0x06` takes a 10-byte payload — purpose unknown.** A length sweep found + that `0x06` rejects every payload length (code `04`) *except* 10, which is + acknowledged with status `00`. Three plausible 10-byte layouts were tried + (`f32 A + f32 A + u16`, `f32 mA + f32 mA + u16`, `u16 mA + u16 mA + u16 + pad`) + and none changed the reported DCR currents. It remains the best candidate for + a DCR- or CAP-parameter block, but it is **not** identified — do not guess at + it in code. +- **CAP cutoff voltage and timer are not available over BLE.** Every opcode was + probed; none reads or writes them. They are front-panel settings (manual + §3.4.2 "CAP Params") and the protocol has no equivalent. The app must not + present them as device controls — see `docs/GUI_DESIGN.md`. +- **`set setpoint` is silently ignored in CAP and DCR.** Sending + `AF 07 03 04 04 ` while in CAP mode is acknowledged with + `DF 07 03 04 01 00 12` — status `00`, identical to the CC-mode + acknowledgement — but changes neither the discharge current nor the cutoff. + Use `0x05` for the CAP discharge current. +- **An ACK is not proof of effect.** The status byte reports that the frame was + well-formed, not that it altered anything. Confirm against a readback + (`0x0A`) or the device display before concluding a command works. +- **Reference capture caveat.** `logs/series_1/btsnoop_hci.log` contains only + six command types, because its CAP step was recorded with factory defaults + (`logs/series_1/protocol.md`, step 9: "Send CAP. Default values."). Absence + from that capture does not mean a command does not exist — `0x05` and `0x0A` + are both absent from it and both work. +- **Grepping the capture:** filter on the write handle, not the payload prefix. + Commands go to handle `0x0009`, notifications arrive on `0x0006`. A 28-byte + status notification fragments as 20+8 on Android, and the 8-byte tail can + begin with `AF` by coincidence, which looks exactly like a command. +- **Characteristics:** the official Android app writes to `FFF3` and subscribes + to `FFF2`. This client uses `FFF1` for both, which the device also accepts — + `Device::connect` picks the first writable/notifying characteristic on the + `FFF0` service rather than a fixed UUID. No command has been found that works + on one characteristic but not the other. - **DCR mode** auto-stops after measurement completes. The `dcr_mohm` field reports resistance in **Ohms** (not milliohms despite the field name). - **Android BLE MTU:** On Android, 28-byte status notifications may arrive diff --git a/docs/GUI_DESIGN.md b/docs/GUI_DESIGN.md index 282c38a..6e0a298 100644 --- a/docs/GUI_DESIGN.md +++ b/docs/GUI_DESIGN.md @@ -74,16 +74,47 @@ Right side — three stacked info cells (mode-dependent): Located in the right column, below the info cards. Hidden for CC/CV/CR/CP modes. **CAP mode (Capacity Test):** -- Line 1: Timer enable/disable toggle. Duration input (HH:MM:SS) is visible only when timer is enabled, on the same line as the Timer toggle. -- Line 2: Cutoff voltage input (always visible, range 0.1–60.0 V) + Chemistry type selector (N/A, NiMH/NiCd, NiZn, Li-Ion, LiPo, LiFePO4, Na-Ion) + Cells count combo box (visible only when chemistry is not N/A; allows picking 1–20 from dropdown or typing any value directly). -- When chemistry is selected, cutoff voltage is auto-calculated as (per-cell voltage × number of cells). -- Per-cell cutoff voltages: NiMH/NiCd=1.00V, NiZn=1.20V, Li-Ion=3.00V, LiPo=3.00V, LiFePO4=2.50V, Na-Ion=2.00V. -- Chemistry/cells selections are persisted in settings. + +Only one CAP parameter is reachable over Bluetooth. The BLE protocol has a dedicated opcode for +the discharge current (write `0x05`, read `0x0A`) but **no command at all** for the cutoff voltage +or the timer — those are front-panel settings. See `docs/BT_PROTOCOL.md`. + +- Line 1: **Discharge current** input (A, range 0–12, i.e. 0–12000 mA) + "Set" button. This is + sent to the device with opcode `0x05`. "Set" is disabled while no device is connected or the + value is out of range. +- Line 2: a static note that Cutoff / Timer are set on the device and are not available over + Bluetooth. +- Line 3 (temporary removed): **"On device: N.NNN A (NNNN mA)"** — the value read back with `0x0A` after each write. + On its own line, not beside the input, so the editable value and the device's value are not + confused. The device stores milliamps, so the readback is quantised (5.0 comes back as + 5.0000010) and must never be compared for exact equality. Shows "—" until the device answers. +- The discharge current is **not** present in the status packet, so it is requested explicitly on + entering CAP mode and after every write. + +*Previously* this panel offered editable Timer, Cutoff voltage, Chemistry and Cells controls. None +of them was ever transmitted to the device, and no BLE command exists that could transmit them — +they silently did nothing. Those editors are commented out in `battery_params_panel` (kept, not +deleted, so they can be restored if a firmware revision exposes the parameters), and their settings +fields are retained as local notes. **DCR mode (DC Internal Resistance Test):** -- I1 current (mA, range 20–12000) -- I2 current (mA, range 20–12000) -- Timer (seconds, range 1–99) + +Like the CAP cutoff, the DCR test currents and timer have **no BLE command** — they are front-panel +settings (manual §3.4.2 "DCR Params"). Unlike the CAP cutoff, they are *readable*: the DCR status +packet carries both test currents. + +- Line 1: a static note that the test currents and timer are set on the device and are not + available over Bluetooth. +- Line 2: read-only **"On device: I1: 20 mA I2: 1000 mA"**, taken from status bytes 15..19 and + 19..23 (Amps on the wire, shown in mA). Shows "—" until the first status packet arrives. + +Both mode panels use the same order — actionable controls, then the device-only notice, then the +read-back values — and share one i18n key for the notice text (`label.device_only_hint`), which is +therefore named after neither mode. + +*Previously* this panel offered editable I1 / I2 / Timer inputs. None was ever transmitted, and no +command exists that could transmit them. Those editors are commented out in `battery_params_panel` +(kept, not deleted) and their settings fields are retained as local notes. ### 5. Chart - V/I/P graph with per-trace toggles (V, I, P colored buttons) @@ -157,7 +188,7 @@ Located in the right column, below the info cards. Hidden for CC/CV/CR/CP modes. | CV | Set Voltage | V | 0.100–60.000 | | CR | Set Resistance | Ω | 0.1–7500.0 | | CP | Set Power | W | 0.00–150.00 | -| CAP | Cutoff V | V | 0.1–60.0 | +| CAP | Discharge current | A | 0.000–12.000 (device range 0–12000 mA) | | DCR | Current | mA | 20–12000 | ### Setpoint Validation diff --git a/el15-app/locales/en.json b/el15-app/locales/en.json index 0b4a7e7..7ee690c 100644 --- a/el15-app/locales/en.json +++ b/el15-app/locales/en.json @@ -29,6 +29,11 @@ "label.timer": "Timer", "label.duration": "Duration", "label.cutoff_v": "Cutoff V", + "label.discharge_current": "Discharge current", + "label.on_device": "On device", + "label.cap_device_only": "Cutoff / Timer", + "label.dcr_device_only": "Test currents / Timer", + "label.device_only_hint": "set on the device — not available over Bluetooth", "label.set_current": "Set Current", "label.set_voltage": "Set Voltage", "label.set_resistance": "Set Resistance", diff --git a/el15-app/locales/es.json b/el15-app/locales/es.json index 3c9277e..5308bd7 100644 --- a/el15-app/locales/es.json +++ b/el15-app/locales/es.json @@ -29,6 +29,11 @@ "label.timer": "Temporizador", "label.duration": "Duración", "label.cutoff_v": "V corte", + "label.discharge_current": "Corriente de descarga", + "label.on_device": "En el dispositivo", + "label.cap_device_only": "Corte / Temporizador", + "label.dcr_device_only": "Corrientes de prueba / Temporizador", + "label.device_only_hint": "se ajustan en el dispositivo: no disponibles por Bluetooth", "label.set_current": "Corriente fijada", "label.set_voltage": "Tensión fijada", "label.set_resistance": "Resistencia fijada", diff --git a/el15-app/locales/hi.json b/el15-app/locales/hi.json index b2e1985..6bad372 100644 --- a/el15-app/locales/hi.json +++ b/el15-app/locales/hi.json @@ -29,6 +29,11 @@ "label.timer": "टाइमर", "label.duration": "अवधि", "label.cutoff_v": "कटऑफ V", + "label.discharge_current": "डिस्चार्ज करंट", + "label.on_device": "डिवाइस पर", + "label.cap_device_only": "कट-ऑफ / टाइमर", + "label.dcr_device_only": "परीक्षण धाराएँ / टाइमर", + "label.device_only_hint": "डिवाइस पर सेट करें — ब्लूटूथ से उपलब्ध नहीं", "label.set_current": "करंट सेट करें", "label.set_voltage": "वोल्टेज सेट करें", "label.set_resistance": "प्रतिरोध सेट करें", diff --git a/el15-app/locales/ru.json b/el15-app/locales/ru.json index 3438701..94af5f2 100644 --- a/el15-app/locales/ru.json +++ b/el15-app/locales/ru.json @@ -29,6 +29,11 @@ "label.timer": "Таймер", "label.duration": "Длительность", "label.cutoff_v": "Отсечка В", + "label.discharge_current": "Ток разряда", + "label.on_device": "На устройстве", + "label.cap_device_only": "Отсечка / Таймер", + "label.dcr_device_only": "Токи измерения / Таймер", + "label.device_only_hint": "задаются на устройстве — недоступны по Bluetooth", "label.set_current": "Задать ток", "label.set_voltage": "Задать напряжение", "label.set_resistance": "Задать сопротивление", diff --git a/el15-app/locales/zh.json b/el15-app/locales/zh.json index f48d64a..fbfbb57 100644 --- a/el15-app/locales/zh.json +++ b/el15-app/locales/zh.json @@ -29,6 +29,11 @@ "label.timer": "计时器", "label.duration": "持续时间", "label.cutoff_v": "截止电压", + "label.discharge_current": "放电电流", + "label.on_device": "设备上", + "label.cap_device_only": "截止电压 / 定时器", + "label.dcr_device_only": "测试电流 / 定时器", + "label.device_only_hint": "在设备上设置 — 蓝牙不支持", "label.set_current": "设定电流", "label.set_voltage": "设定电压", "label.set_resistance": "设定电阻", diff --git a/el15-app/src/cli_run.rs b/el15-app/src/cli_run.rs index 7711c21..45ae2ca 100644 --- a/el15-app/src/cli_run.rs +++ b/el15-app/src/cli_run.rs @@ -123,6 +123,7 @@ async fn try_connect(args: &Cli, state: &SharedState) -> Result> { match ev { DeviceEvent::Status(s) => st_clone.update_status(s).await, DeviceEvent::FirmwareVersion(ver) => info!("firmware version: {ver}"), + DeviceEvent::CapCurrent(amps) => info!("CAP discharge current: {amps} A"), DeviceEvent::RawNotification(_) => {} DeviceEvent::Disconnected => { warn!("device disconnected"); @@ -220,6 +221,9 @@ async fn run_debug_shell(args: &Cli) -> Result<()> { DeviceEvent::FirmwareVersion(ver) => { println!(" << FIRMWARE VERSION: {ver}"); } + DeviceEvent::CapCurrent(amps) => { + println!(" << CAP DISCHARGE CURRENT: {amps:.4} A ({:.0} mA)", amps * 1000.0); + } DeviceEvent::Disconnected => { println!(" << DISCONNECTED"); break; diff --git a/el15-app/src/gui.rs b/el15-app/src/gui.rs index 0916897..74f9121 100644 --- a/el15-app/src/gui.rs +++ b/el15-app/src/gui.rs @@ -30,8 +30,9 @@ use tokio_stream::wrappers::UnboundedReceiverStream; use tracing::{debug, info, warn}; use el15_bt::{ - build_mode_cmd, build_set_setpoint_cmd, scan_devices, scan_for_device, Device, DeviceEvent, - DeviceInfo, EL15Status, Mode, CMD_LOAD_OFF, CMD_LOAD_ON, + build_mode_cmd, build_set_cap_current_cmd, build_set_setpoint_cmd, scan_devices, + scan_for_device, Device, DeviceEvent, DeviceInfo, EL15Status, Mode, CAP_CURRENT_MAX_A, + CMD_GET_CAP_CURRENT, CMD_LOAD_OFF, CMD_LOAD_ON, INTER_COMMAND_GAP, }; use el15_scpi::{ScpiServer, ScpiServerConfig, SharedState as ScpiSharedState}; @@ -145,6 +146,8 @@ pub enum Message { CapTimerToggle, CapTimerChanged(String), CapCutoffChanged(String), + CapCurrentChanged(String), + ApplyCapCurrent, CapRecordClear, CapChemistryChanged(String), CapCellsChanged(String), @@ -227,10 +230,18 @@ pub struct AppState { chart_height: f32, graph_time_input: String, graph_retention_input: String, + /// CAP discharge current last read back from the device (Amps), from a + /// `DF 07 03 0A` reply. `None` until the device answers — it is never + /// present in the periodic status packet. + device_cap_current: Option, /// Epoch set by the graph's **Clear** button: samples older than this are /// hidden from the graph but kept in the buffer for CSV export. Set here /// and nowhere else, so nothing but Clear can hide recorded data. graph_start_time: Option>, + /// Retained for the CAP cells selector, which is commented out in + /// `battery_params_panel` because the cutoff it feeds cannot be sent + /// over BLE. Kept so that editor can be restored as-is. + #[allow(dead_code)] cells_combo_state: combo_box::State, // ---- flash / DFU page ---- @@ -320,6 +331,7 @@ impl AppState { chart_height: 160.0, graph_time_input: time_window_str, graph_retention_input: retention_str, + device_cap_current: None, graph_start_time: None, cells_combo_state: combo_box::State::new((1u8..=20).map(|n| n.to_string()).collect()), show_flash_page: false, @@ -374,6 +386,19 @@ impl AppState { format!("{} v{}", t!("app.title"), env!("CARGO_PKG_VERSION")) } + /// How many poll ticks to skip while a burst of `frames` commands is sent. + /// + /// A burst takes `(frames - 1) * INTER_COMMAND_GAP` to drain, and a poll + /// landing in the middle of it would be one more back-to-back write — the + /// very thing the gap exists to prevent. Scaled to the poll interval so a + /// fast poll rate does not resume early. + fn pause_ticks_for(&self, frames: usize) -> u8 { + let burst_ms = frames.saturating_sub(1) as u64 * INTER_COMMAND_GAP.as_millis() as u64; + let interval = self.settings.poll_interval_ms.max(1); + // +1 so at least one tick is always skipped, and one more for slack. + (burst_ms.div_ceil(interval) + 2).min(u8::MAX as u64) as u8 + } + /// Upper bound on the shared sample buffer at the current poll rate. fn sample_capacity(&self) -> usize { settings::sample_capacity( @@ -450,15 +475,14 @@ impl AppState { if secs >= bucket_start && secs < bucket_start + tick_ms { if let Some(dev) = self.device.clone() { info!("retrying mode switch to {:?} (elapsed {}ms)", target, secs); - self.pause_poll_ticks = 3; - let mode = target.to_proto(); - let setpoint = stored_setpoint(&self.settings, target); + let mut frames = vec![build_mode_cmd(target.to_proto())]; + if let Some(sp) = stored_setpoint(&self.settings, target) { + frames.push(build_set_setpoint_cmd(sp)); + } + self.pause_poll_ticks = self.pause_ticks_for(frames.len()); return Task::perform( async move { - let _ = dev.send(&build_mode_cmd(mode)).await; - if let Some(sp) = setpoint { - let _ = dev.send(&build_set_setpoint_cmd(sp)).await; - } + let _ = dev.send_sequence(&frames).await; Message::Noop }, |m| m, @@ -761,6 +785,10 @@ impl AppState { DeviceEvent::FirmwareVersion(ver) => { self.firmware_version = Some(ver); } + DeviceEvent::CapCurrent(amps) => { + debug!("device CAP discharge current: {amps} A"); + self.device_cap_current = Some(amps); + } DeviceEvent::RawNotification(_) => {} DeviceEvent::Disconnected => { // If device is already None, this is a stale notification from @@ -798,15 +826,20 @@ impl AppState { // the mode command and the concurrent poll don't race on the // same BLE characteristic (concurrent writes are often dropped). self.pause_poll_ticks = 3; - let mode = mk.to_proto(); - let setpoint = stored_setpoint(&self.settings, mk); + let mut frames = vec![build_mode_cmd(mk.to_proto())]; + // Also send the stored setpoint so device uses our value. + if let Some(sp) = stored_setpoint(&self.settings, mk) { + frames.push(build_set_setpoint_cmd(sp)); + } + // CAP's discharge current never appears in the status + // packet, so it must be requested explicitly to be shown. + if mk == ModeKind::CAP { + frames.push(CMD_GET_CAP_CURRENT.to_vec()); + } + self.pause_poll_ticks = self.pause_ticks_for(frames.len()); return Task::perform( async move { - let _ = dev.send(&build_mode_cmd(mode)).await; - // Also send the stored setpoint so device uses our value - if let Some(sp) = setpoint { - let _ = dev.send(&build_set_setpoint_cmd(sp)).await; - } + let _ = dev.send_sequence(&frames).await; Message::Noop }, |m| m, @@ -851,40 +884,51 @@ impl AppState { info!("toggle load -> {}", if want_on {"ON"} else {"OFF"}); let bytes = if want_on { CMD_LOAD_ON } else { CMD_LOAD_OFF }; if let Some(dev) = self.device.clone() { - // Pause polls so the load command doesn't race on the - // same BLE characteristic (same as SetMode). - self.pause_poll_ticks = 3; - // When turning load ON, re-send mode + setpoint to ensure - // the device uses the user-selected mode (guards against - // a lost earlier mode command). - let mode_cmd = if want_on { - Some(build_mode_cmd(self.settings.last_mode.to_proto())) - } else { - None - }; - // When turning load ON, parse text input (user may not have pressed Set) - let setpoint = if want_on { - let mode = self.settings.last_mode; - if let Ok(v) = self.setpoint_input.parse::() { - let v = clamp_setpoint(mode, v); - store_setpoint(&mut self.settings, mode, v); - let _ = settings::save(&self.settings); - Some(v) - } else { - stored_setpoint(&self.settings, mode) + let mode = self.settings.last_mode; + let mut frames: Vec> = Vec::new(); + if want_on { + // Re-send mode + parameter to ensure the device uses the + // user-selected mode (guards against a lost earlier + // mode command). + frames.push(build_mode_cmd(mode.to_proto())); + match mode { + // CC/CV/CR/CP: the setpoint may not have been + // applied yet, so take it from the text input. + ModeKind::CC | ModeKind::CV | ModeKind::CR | ModeKind::CP => { + let sp = match self.setpoint_input.parse::() { + Ok(v) => { + let v = clamp_setpoint(mode, v); + store_setpoint(&mut self.settings, mode, v); + let _ = settings::save(&self.settings); + Some(v) + } + Err(_) => stored_setpoint(&self.settings, mode), + }; + if let Some(sp) = sp { + frames.push(build_set_setpoint_cmd(sp)); + } + } + // CAP has its own opcode. Sending the ordinary + // setpoint here used to push a stale value from + // `setpoint_input` that the device silently ignores, + // wasting a slot in the burst and costing the load + // command its turn. + ModeKind::CAP => { + if let Ok(a) = self.settings.cap.current_input.parse::() { + frames.push(build_set_cap_current_cmd(a)); + } + } + // DCR takes its parameters from the front panel. + ModeKind::DCR => {} } - } else { - None - }; + } + frames.push(bytes.to_vec()); + // Pause polls so the burst doesn't race one on the same + // characteristic (same as SetMode). + self.pause_poll_ticks = self.pause_ticks_for(frames.len()); return Task::perform( async move { - if let Some(cmd) = mode_cmd { - let _ = dev.send(&cmd).await; - } - if let Some(sp) = setpoint { - let _ = dev.send(&build_set_setpoint_cmd(sp)).await; - } - let _ = dev.send(&bytes).await; + let _ = dev.send_sequence(&frames).await; Message::Noop }, |m| m, @@ -1178,9 +1222,38 @@ impl AppState { let _ = settings::save(&self.settings); } Message::CapCutoffChanged(v) => { + // Local note only — the device has no BLE command for the CAP + // cutoff voltage, so this value is never transmitted. self.settings.cap.cutoff_input = v; let _ = settings::save(&self.settings); } + Message::CapCurrentChanged(v) => { + self.settings.cap.current_input = v; + } + Message::ApplyCapCurrent => { + let Ok(amps) = self.settings.cap.current_input.parse::() else { + return Task::none(); + }; + let amps = amps.clamp(0.0, CAP_CURRENT_MAX_A); + self.settings.cap.current_input = format!("{amps:.3}"); + let _ = settings::save(&self.settings); + if let Some(dev) = self.device.clone() { + // Write, then read straight back: the device quantises to + // milliamps, so the stored value is the truth. + let frames = vec![ + build_set_cap_current_cmd(amps), + CMD_GET_CAP_CURRENT.to_vec(), + ]; + self.pause_poll_ticks = self.pause_ticks_for(frames.len()); + return Task::perform( + async move { + let _ = dev.send_sequence(&frames).await; + Message::Noop + }, + |m| m, + ); + } + } Message::CapRecordClear => { self.samples.clear(); self.graph_start_time = None; @@ -1217,12 +1290,15 @@ impl AppState { let _ = settings::save(&self.settings); } Message::DcrStart => { - // DCR mode: ensure mode is set, then enable load to start test + // DCR mode: ensure mode is set, then enable load to start test. if let Some(dev) = self.device.clone() { + // Paced: sent back-to-back, the load command is dropped and + // the test never starts until the button is pressed again. + let frames = vec![build_mode_cmd(Mode::DCR), CMD_LOAD_ON.to_vec()]; + self.pause_poll_ticks = self.pause_ticks_for(frames.len()); return Task::perform( async move { - let _ = dev.send(&build_mode_cmd(Mode::DCR)).await; - let _ = dev.send(&CMD_LOAD_ON).await; + let _ = dev.send_sequence(&frames).await; Message::Noop }, |m| m, @@ -1842,73 +1918,141 @@ impl AppState { } match self.settings.last_mode { ModeKind::CAP => { - let timer_btn_label = if self.settings.cap.timer_enabled { t!("btn.disable").to_string() } else { t!("btn.enable").to_string() }; - let timer_state = if self.settings.cap.timer_enabled { t!("btn.load_on").to_string() } else { t!("btn.load_off").to_string() }; - - // Line 1: Timer toggle + Duration (only shown when timer enabled) - let mut timer_row = row![ - text(format!("{}:", t!("label.timer"))).size(12), - text(timer_state).size(12), - Space::new().width(8.0), - button(text(timer_btn_label).size(11)) - .padding([3, 8]) - .on_press(Message::CapTimerToggle), - ].spacing(6).align_y(iced::Alignment::Center); - - if self.settings.cap.timer_enabled { - timer_row = timer_row.push(Space::new().width(16.0)); - timer_row = timer_row.push(text(format!("{}:", t!("label.duration"))).size(12)); - timer_row = timer_row.push( - text_input("01:00:00", &self.settings.cap.timer_input) - .on_input(Message::CapTimerChanged) - .width(Length::Fixed(90.0)) - .size(13), - ); + // Of the device's CAP parameters, only the discharge current is + // reachable over BLE (write `0x05`, read `0x0A`). Verified on + // HW:2.0 / SW:1.7 — see `docs/BT_PROTOCOL.md`. + // + // The cutoff voltage and the timer have **no** BLE command at + // all: every opcode in the device's command space was probed and + // none of them writes either value. The editors for them are + // commented out below rather than deleted, so they can be + // restored the day a firmware revision exposes them. Leaving + // them enabled was the original bug — they looked like device + // controls but were never transmitted anywhere. + + // Line 1: discharge current — editable, and actually sent. + let current_valid = self + .settings + .cap + .current_input + .parse::() + .is_ok_and(|a| (0.0..=CAP_CURRENT_MAX_A).contains(&a)); + let mut set_current_btn = + button(text(t!("btn.set")).size(11)).padding([3, 8]); + if current_valid && self.device.is_some() { + set_current_btn = set_current_btn.on_press(Message::ApplyCapCurrent); } - - // Line 2: Cutoff V + Chemistry + Cells - let chemistry_display = if self.settings.cap.chemistry.is_empty() { - t!("label.na").to_string() - } else { - self.settings.cap.chemistry.clone() - }; - - let has_chemistry = !self.settings.cap.chemistry.is_empty() - && self.settings.cap.chemistry != t!("label.na").as_ref(); - - let mut cutoff_row = row![ - text(format!("{}:", t!("label.cutoff_v"))).size(12), - text_input("3.0", &self.settings.cap.cutoff_input) - .on_input(Message::CapCutoffChanged) - .width(Length::Fixed(60.0)) + let current_row = row![ + text(format!("{}:", t!("label.discharge_current"))).size(12), + text_input("1.000", &self.settings.cap.current_input) + .on_input(Message::CapCurrentChanged) + .on_submit(Message::ApplyCapCurrent) + .width(Length::Fixed(70.0)) .size(13), - text("V").size(12), - Space::new().width(16.0), - text(format!("{}:", t!("label.chemistry_type"))).size(12), - pick_list( - chemistry_names(), - Some(chemistry_display), - Message::CapChemistryChanged, - ).text_size(12), - ].spacing(6).align_y(iced::Alignment::Center); - - if has_chemistry { - let cells_str = self.settings.cap.cells.to_string(); - let cells_selected = Some(&cells_str); - cutoff_row = cutoff_row.push(Space::new().width(12.0)); - cutoff_row = cutoff_row.push( - combo_box(&self.cells_combo_state, "#", cells_selected, Message::CapCellsChanged) - .on_input(Message::CapCellsChanged) - .width(Length::Fixed(75.0)) - .size(13.0), - ); - } + text("A").size(12), + set_current_btn, + ] + .spacing(6) + .align_y(iced::Alignment::Center); + + // Line 2: the parameters the protocol cannot reach. + let device_only_row = row![ + text(format!( + "{}: {}", + t!("label.cap_device_only"), + t!("label.device_only_hint") + )) + .size(11), + ] + .spacing(6) + .align_y(iced::Alignment::Center); + + // // Line 3: what the device actually holds, read back after every + // // write. On its own line rather than beside the input, so the + // // editable value and the device's value are not confused. + // let on_device_row = row![ + // text(match self.device_cap_current { + // Some(a) => format!( + // "{}: {:.3} A ({:.0} mA)", + // t!("label.on_device"), + // a, + // a * 1000.0 + // ), + // None => format!("{}: —", t!("label.on_device")), + // }) + // .size(12), + // ] + // .spacing(6) + // .align_y(iced::Alignment::Center); + + // ---- Not sendable over BLE — kept for a future firmware ---- + // + // let timer_btn_label = if self.settings.cap.timer_enabled { t!("btn.disable").to_string() } else { t!("btn.enable").to_string() }; + // let timer_state = if self.settings.cap.timer_enabled { t!("btn.load_on").to_string() } else { t!("btn.load_off").to_string() }; + // + // let mut timer_row = row![ + // text(format!("{}:", t!("label.timer"))).size(12), + // text(timer_state).size(12), + // Space::new().width(8.0), + // button(text(timer_btn_label).size(11)) + // .padding([3, 8]) + // .on_press(Message::CapTimerToggle), + // ].spacing(6).align_y(iced::Alignment::Center); + // + // if self.settings.cap.timer_enabled { + // timer_row = timer_row.push(Space::new().width(16.0)); + // timer_row = timer_row.push(text(format!("{}:", t!("label.duration"))).size(12)); + // timer_row = timer_row.push( + // text_input("01:00:00", &self.settings.cap.timer_input) + // .on_input(Message::CapTimerChanged) + // .width(Length::Fixed(90.0)) + // .size(13), + // ); + // } + // + // let chemistry_display = if self.settings.cap.chemistry.is_empty() { + // t!("label.na").to_string() + // } else { + // self.settings.cap.chemistry.clone() + // }; + // + // let has_chemistry = !self.settings.cap.chemistry.is_empty() + // && self.settings.cap.chemistry != t!("label.na").as_ref(); + // + // let mut cutoff_row = row![ + // text(format!("{}:", t!("label.cutoff_v"))).size(12), + // text_input("3.0", &self.settings.cap.cutoff_input) + // .on_input(Message::CapCutoffChanged) + // .width(Length::Fixed(60.0)) + // .size(13), + // text("V").size(12), + // Space::new().width(16.0), + // text(format!("{}:", t!("label.chemistry_type"))).size(12), + // pick_list( + // chemistry_names(), + // Some(chemistry_display), + // Message::CapChemistryChanged, + // ).text_size(12), + // ].spacing(6).align_y(iced::Alignment::Center); + // + // if has_chemistry { + // let cells_str = self.settings.cap.cells.to_string(); + // let cells_selected = Some(&cells_str); + // cutoff_row = cutoff_row.push(Space::new().width(12.0)); + // cutoff_row = cutoff_row.push( + // combo_box(&self.cells_combo_state, "#", cells_selected, Message::CapCellsChanged) + // .on_input(Message::CapCellsChanged) + // .width(Length::Fixed(75.0)) + // .size(13.0), + // ); + // } container( column![ text(t!("label.cap_params").to_string()).size(13), - timer_row, - cutoff_row, + current_row, + device_only_row, + // on_device_row, ] .spacing(6), ) @@ -1918,32 +2062,40 @@ impl AppState { .into() } ModeKind::DCR => { + // Like the CAP cutoff, the DCR test currents and timer have no + // BLE command — they are front-panel settings (manual §3.4.2, + // "DCR Params"). Verified on HW:2.0 / SW:1.7: neither `0x04` + // nor `0x05` moves them, and they are the only writable opcodes + // whose payload shape is known. The editors below are commented + // out rather than deleted; see `docs/BT_PROTOCOL.md`. + // + // They *are* readable, though: the status packet reports both + // test currents in DCR mode, so show what the device holds. + let (i1, i2) = match self.last_status.as_ref() { + Some(st) => ( + format!("{:.0} mA", st.dcr_i1 * 1000.0), + format!("{:.0} mA", st.dcr_i2 * 1000.0), + ), + None => ("—".to_string(), "—".to_string()), + }; + container( column![ text(t!("label.dcr_params").to_string()).size(13), + // Same order as the CAP panel: the device-only notice + // first, then the read-back values on their own line. + text(format!( + "{}: {}", + t!("label.dcr_device_only"), + t!("label.device_only_hint") + )).size(11), row![ - text("I1: ").size(12), - text_input("20", &self.settings.dcr.i1_input) - .on_input(Message::DcrI1Changed) - .width(Length::Fixed(40.0)) - .size(13), - text("mA").size(12), - Space::new().width(12.0), - text("I2: ").size(12), - text_input("1000", &self.settings.dcr.i2_input) - .on_input(Message::DcrI2Changed) - .width(Length::Fixed(60.0)) - .size(13), - text("mA").size(12) + text(format!("{}:", t!("label.on_device"))).size(12), + Space::new().width(6.0), + text(format!("I1: {i1}")).size(12), + Space::new().width(16.0), + text(format!("I2: {i2}")).size(12), ].align_y(iced::Alignment::Center), - row![ - text(format!("{}:", t!("label.timer"))).size(12), - text_input("2", &self.settings.dcr.timer_input) - .on_input(Message::DcrTimerChanged) - .width(Length::Fixed(30.0)) - .size(13), - text("s").size(12), - ].spacing(6).align_y(iced::Alignment::Center), ] .spacing(6), ) @@ -1951,6 +2103,32 @@ impl AppState { .style(container::bordered_box) .width(Length::Fill) .into() + + // ---- Not sendable over BLE — kept for a future firmware ---- + // + // row![ + // text("I1: ").size(12), + // text_input("20", &self.settings.dcr.i1_input) + // .on_input(Message::DcrI1Changed) + // .width(Length::Fixed(40.0)) + // .size(13), + // text("mA").size(12), + // Space::new().width(12.0), + // text("I2: ").size(12), + // text_input("1000", &self.settings.dcr.i2_input) + // .on_input(Message::DcrI2Changed) + // .width(Length::Fixed(60.0)) + // .size(13), + // text("mA").size(12) + // ].align_y(iced::Alignment::Center), + // row![ + // text(format!("{}:", t!("label.timer"))).size(12), + // text_input("2", &self.settings.dcr.timer_input) + // .on_input(Message::DcrTimerChanged) + // .width(Length::Fixed(30.0)) + // .size(13), + // text("s").size(12), + // ].spacing(6).align_y(iced::Alignment::Center), } _ => Space::new().height(0.0).into(), } @@ -2598,6 +2776,9 @@ const CHEMISTRY_TYPES: &[(&str, f32)] = &[ ("Na-Ion", 2.00), ]; +/// Retained for the commented-out CAP chemistry selector — see +/// `battery_params_panel`. The cutoff voltage it derives has no BLE command. +#[allow(dead_code)] fn chemistry_names() -> Vec { let mut v = vec![t!("label.na").to_string()]; v.extend(CHEMISTRY_TYPES.iter().map(|(name, _)| name.to_string())); @@ -2697,6 +2878,7 @@ mod tests { chart_height: 160.0, graph_time_input: time_window_str, graph_retention_input: retention_str, + device_cap_current: None, graph_start_time: None, cells_combo_state: combo_box::State::new( (1u8..=20).map(|n| n.to_string()).collect(), @@ -2968,6 +3150,117 @@ mod tests { ); } + /// The CAP discharge current is the one CAP parameter the protocol can + /// write, so the input must normalise and clamp to the device's range. + #[test] + fn cap_current_is_normalised_and_clamped_on_apply() { + let mut state = test_state(); + + state.settings.cap.current_input = "2.5".to_string(); + let _ = state.update(Message::ApplyCapCurrent); + assert_eq!(state.settings.cap.current_input, "2.500"); + + state.settings.cap.current_input = "99".to_string(); + let _ = state.update(Message::ApplyCapCurrent); + assert_eq!(state.settings.cap.current_input, "12.000"); + + state.settings.cap.current_input = "-3".to_string(); + let _ = state.update(Message::ApplyCapCurrent); + assert_eq!(state.settings.cap.current_input, "0.000"); + } + + #[test] + fn cap_current_apply_ignores_unparseable_input() { + let mut state = test_state(); + state.settings.cap.current_input = "abc".to_string(); + let _ = state.update(Message::ApplyCapCurrent); + // Left untouched rather than silently coerced to 0 A. + assert_eq!(state.settings.cap.current_input, "abc"); + } + + /// The device only reports this value when asked; it is never in a status + /// packet, so the readback has to come from the dedicated event. + #[test] + fn cap_current_event_updates_the_readback() { + let mut state = test_state(); + assert!(state.device_cap_current.is_none()); + let _ = state.update(Message::DeviceEvent(DeviceEvent::CapCurrent(5.000001))); + assert_eq!(state.device_cap_current, Some(5.000001)); + } + + /// Regression guard for the original bug: the cutoff voltage is a local + /// note only, so changing it must not produce any device traffic. + #[test] + fn cap_cutoff_is_local_only() { + let mut state = test_state(); + let _ = state.update(Message::CapCutoffChanged("2.8".to_string())); + assert_eq!(state.settings.cap.cutoff_input, "2.8"); + // No device is connected in tests; the point is that no send path + // exists for it at all — `stored_setpoint` also refuses CAP. + assert!(stored_setpoint(&state.settings, ModeKind::CAP).is_none()); + } + + /// A burst must stay paused for at least as long as it takes to drain, + /// otherwise a poll lands in the middle of it and becomes the very + /// back-to-back write the inter-command gap exists to avoid. + #[test] + fn poll_pause_covers_the_whole_burst() { + let mut state = test_state(); + let gap_ms = INTER_COMMAND_GAP.as_millis() as u64; + + for interval in [50_u64, 100, 200, 500, 1000] { + state.settings.poll_interval_ms = interval; + for frames in 1..=4_usize { + let ticks = state.pause_ticks_for(frames) as u64; + let burst_ms = (frames as u64 - 1) * gap_ms; + assert!( + ticks * interval >= burst_ms, + "interval {interval}ms, {frames} frames: paused {}ms < burst {burst_ms}ms", + ticks * interval + ); + assert!(ticks >= 1, "must always skip at least one poll"); + } + } + } + + /// Regression guard for the multiple-clicks bug: in CAP mode the load-on + /// burst used to carry a stale `0x04` setpoint that the device ignores, + /// which cost the load command its slot in the burst. + #[test] + fn cap_load_on_burst_uses_the_cap_opcode_not_the_setpoint() { + let settings = Settings::default(); + // What the old code would have sent for CAP: a setpoint from the text + // input. `stored_setpoint` already refuses CAP — the bug was that + // ToggleLoad bypassed it by parsing `setpoint_input` directly. + assert!(stored_setpoint(&settings, ModeKind::CAP).is_none()); + assert!(stored_setpoint(&settings, ModeKind::DCR).is_none()); + + // The CAP parameter frame must be the 0x05 opcode, not 0x04. + let cap_frame = build_set_cap_current_cmd(5.0); + assert_eq!(cap_frame[3], 0x05); + let setpoint_frame = build_set_setpoint_cmd(5.0); + assert_eq!(setpoint_frame[3], 0x04); + assert_ne!(cap_frame, setpoint_frame); + } + + /// DCR's test currents and timer have no BLE command, so nothing in the + /// DCR path may send a parameter frame. `stored_setpoint` refusing DCR is + /// what `ToggleLoad` now relies on; the panel itself is read-only. + #[test] + fn dcr_params_are_never_sent() { + let mut state = test_state(); + let _ = state.update(Message::DcrI1Changed("50".to_string())); + let _ = state.update(Message::DcrI2Changed("500".to_string())); + let _ = state.update(Message::DcrTimerChanged("3".to_string())); + + // Retained as local notes... + assert_eq!(state.settings.dcr.i1_input, "50"); + assert_eq!(state.settings.dcr.i2_input, "500"); + assert_eq!(state.settings.dcr.timer_input, "3"); + // ...but there is no send path for them. + assert!(stored_setpoint(&state.settings, ModeKind::DCR).is_none()); + } + #[test] fn duration_labels_are_compact() { assert_eq!(fmt_duration_short(0), "0s"); diff --git a/el15-app/src/settings.rs b/el15-app/src/settings.rs index fc05380..d159339 100644 --- a/el15-app/src/settings.rs +++ b/el15-app/src/settings.rs @@ -108,6 +108,12 @@ impl Default for GraphSettings { } } +/// Capacity-test settings. +/// +/// Only `current_input` reaches the device. The BLE protocol exposes the CAP +/// discharge current (write `0x05`, read `0x0A`) but has **no** command for the +/// cutoff voltage or the timer — those are front-panel settings. The remaining +/// fields are therefore kept as local notes only; see `docs/BT_PROTOCOL.md`. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CapSettings { pub timer_enabled: bool, @@ -117,10 +123,15 @@ pub struct CapSettings { pub chemistry: String, #[serde(default = "default_cells")] pub cells: u8, + /// Discharge current in Amps, as typed by the user. Sent to the device. + #[serde(default = "default_cap_current")] + pub current_input: String, } fn default_cells() -> u8 { 1 } +fn default_cap_current() -> String { "1.000".to_string() } + impl Default for CapSettings { fn default() -> Self { Self { @@ -129,6 +140,7 @@ impl Default for CapSettings { cutoff_input: "3.0".to_string(), chemistry: String::new(), cells: 1, + current_input: default_cap_current(), } } } diff --git a/el15-bt/src/device.rs b/el15-bt/src/device.rs index 1856505..78cad7c 100644 --- a/el15-bt/src/device.rs +++ b/el15-bt/src/device.rs @@ -25,7 +25,10 @@ use tracing::{debug, info}; use uuid::{uuid, Uuid}; use crate::error::{Error, Result}; -use crate::protocol::{parse_status_packet, parse_firmware_version, EL15Status, HEADER, POLL_PKT, CMD_INIT, CMD_INFO}; +use crate::protocol::{ + parse_cap_current, parse_status_packet, parse_firmware_version, EL15Status, HEADER, POLL_PKT, + CMD_INIT, CMD_INFO, CMD_GET_CAP_CURRENT, +}; const SCAN_DURATION: Duration = Duration::from_secs(5); @@ -33,6 +36,22 @@ const SCAN_DURATION: Duration = Duration::from_secs(5); /// The writable and notify characteristics live under this service. pub const EL15_SERVICE_UUID: Uuid = uuid!("0000fff0-0000-1000-8000-00805f9b34fb"); +/// Minimum gap between two consecutive command writes. +/// +/// The write characteristic advertises WRITE_WITHOUT_RESPONSE, so [`Device::send`] +/// uses it — and that has no flow control: frames written back-to-back are +/// silently dropped, with no error on this side. +/// +/// Measured on HW:2.0 / SW:1.7. Three commands sent with no gap (set mode, set +/// setpoint, set CAP current) landed **none** of the trailing two — the CAP +/// current read back unchanged three times running, and one reply came back +/// corrupted (`df 01 04 39 …`, containing bytes of the outgoing frame). The +/// same three spaced 120 ms apart took effect 3 times out of 3. +/// +/// This is why a burst of commands used to need several button presses before +/// one "took". +pub const INTER_COMMAND_GAP: Duration = Duration::from_millis(120); + /// Friendly name prefixes used as a fallback when advertisement data does not /// carry the service UUID list (some BT stacks strip 128-bit UUIDs from the /// passive scan response). @@ -71,6 +90,11 @@ fn short_id(id: &str) -> String { pub enum DeviceEvent { Status(EL15Status), FirmwareVersion(String), + /// CAP discharge current in Amps, from a `DF 07 03 0A` response. + /// + /// Only emitted in reply to [`Device::request_cap_current`]; the device + /// never reports this value in the periodic status packet. + CapCurrent(f32), RawNotification(Vec), Disconnected, } @@ -326,6 +350,8 @@ impl Device { } } else if let Some(ver) = parse_firmware_version(&data) { let _ = tx.send(DeviceEvent::FirmwareVersion(ver)).await; + } else if let Some(amps) = parse_cap_current(&data) { + let _ = tx.send(DeviceEvent::CapCurrent(amps)).await; } } let _ = tx.send(DeviceEvent::Disconnected).await; @@ -357,10 +383,33 @@ impl Device { Ok(()) } + /// Send several commands in order, pausing [`INTER_COMMAND_GAP`] between them. + /// + /// Always prefer this over consecutive [`Device::send`] calls: commands + /// written back-to-back are silently dropped (see the constant's docs). + pub async fn send_sequence(&self, frames: &[Vec]) -> Result<()> { + for (i, frame) in frames.iter().enumerate() { + if i > 0 { + tokio::time::sleep(INTER_COMMAND_GAP).await; + } + self.send(frame).await?; + } + Ok(()) + } + pub async fn poll(&self) -> Result<()> { self.send(&POLL_PKT).await } + /// Ask the device for its CAP discharge current. + /// + /// The answer arrives asynchronously as [`DeviceEvent::CapCurrent`]. This + /// is the only way to observe the value — CAP status packets carry the + /// measured capacity in the field that holds the setpoint in other modes. + pub async fn request_cap_current(&self) -> Result<()> { + self.send(&CMD_GET_CAP_CURRENT).await + } + /// Send the init handshake and info request to wake up the device's /// full status reporting (temperature, setpoint, etc.). pub async fn init_handshake(&self) -> Result<()> { diff --git a/el15-bt/src/lib.rs b/el15-bt/src/lib.rs index 3181ba5..40f5d27 100644 --- a/el15-bt/src/lib.rs +++ b/el15-bt/src/lib.rs @@ -11,11 +11,13 @@ pub mod error; pub use error::{Error, Result}; pub use protocol::{ EL15Status, Mode, MODE_NAMES, HEADER, POLL_PKT, - CMD_LOAD_ON, CMD_LOAD_OFF, CMD_LOCK, CMD_INIT, CMD_INFO, - build_set_setpoint_cmd, build_mode_cmd, parse_status_packet, checksum, + CMD_LOAD_ON, CMD_LOAD_OFF, CMD_LOCK, CMD_INIT, CMD_INFO, CMD_GET_CAP_CURRENT, + CAP_CURRENT_MAX_A, + build_set_setpoint_cmd, build_set_cap_current_cmd, build_mode_cmd, + parse_status_packet, parse_cap_current, checksum, parse_firmware_version, }; pub use device::{ scan_devices, scan_devices_with, scan_for_device, Device, DeviceEvent, DeviceInfo, ScanOptions, - EL15_SERVICE_UUID, + EL15_SERVICE_UUID, INTER_COMMAND_GAP, }; diff --git a/el15-bt/src/protocol.rs b/el15-bt/src/protocol.rs index 8c9b69f..c0f6a9f 100644 --- a/el15-bt/src/protocol.rs +++ b/el15-bt/src/protocol.rs @@ -29,8 +29,28 @@ pub const CMD_LOCK: [u8; 7] = [0xAF, 0x07, 0x03, 0x09, 0x01, 0x01, 0x3C]; const CMD_MODE_PREFIX: [u8; 5] = [0xAF, 0x07, 0x03, 0x03, 0x01]; /// Prefix of a "set setpoint" command — append the f32 LE bytes + checksum. +/// +/// Applies to CC/CV/CR/CP only. In CAP and DCR the device acknowledges this +/// command with status `0x00` and then ignores it — use [`CMD_SET_CAP_CURRENT_PREFIX`] +/// for the CAP discharge current. const CMD_SETPOINT_PREFIX: [u8; 5] = [0xAF, 0x07, 0x03, 0x04, 0x04]; +/// Prefix of a "set CAP discharge current" command (cmd `0x05`, f32 LE Amps). +/// +/// Separate opcode from the ordinary setpoint. Verified on HW:2.0 / SW:1.7: +/// writing here changes the value that [`CMD_GET_CAP_CURRENT`] reads back and +/// the current shown on the device's capacity-test screen. +const CMD_SET_CAP_CURRENT_PREFIX: [u8; 5] = [0xAF, 0x07, 0x03, 0x05, 0x04]; + +/// Read the CAP discharge current — device replies `DF 07 03 0A 04 `. +/// +/// The device applies its own quantisation (it stores milliamps), so a value +/// written as `5.0` reads back as `5.0000010`. +pub const CMD_GET_CAP_CURRENT: [u8; 6] = [0xAF, 0x07, 0x03, 0x0A, 0x00, 0x3D]; + +/// Header of the CAP discharge-current response. +pub const CAP_CURRENT_HEADER: [u8; 5] = [0xDF, 0x07, 0x03, 0x0A, 0x04]; + /// Compute the checksum byte that makes `sum(packet) % 256 == 0`. pub fn checksum(data: &[u8]) -> u8 { let sum: u8 = data.iter().fold(0u8, |acc, &b| acc.wrapping_add(b)); @@ -316,6 +336,31 @@ pub fn build_set_setpoint_cmd(value: f32) -> Vec { v } +/// Highest CAP discharge current the device accepts (12 A = 12000 mA). +pub const CAP_CURRENT_MAX_A: f32 = 12.0; + +/// Build a "set CAP discharge current" command (`0x05`), value in **Amps**. +/// +/// The value is clamped to the device's documented `0 – 12000 mA` range. +pub fn build_set_cap_current_cmd(amps: f32) -> Vec { + let amps = amps.clamp(0.0, CAP_CURRENT_MAX_A); + let mut v = Vec::with_capacity(10); + v.extend_from_slice(&CMD_SET_CAP_CURRENT_PREFIX); + v.extend_from_slice(&s.to_le_bytes()); + v.push(checksum(&v)); + v +} + +/// Parse a CAP discharge-current response (`DF 07 03 0A 04 `). +/// +/// Returns the current in Amps, or `None` if this is not that response. +pub fn parse_cap_current(data: &[u8]) -> Option { + if data.len() < 10 || data[..5] != CAP_CURRENT_HEADER { + return None; + } + Some(f32::from_le_bytes([data[5], data[6], data[7], data[8]])) +} + /// Parse the firmware version from the Init response notification. /// /// Packet: `DF FF FF ` (8 bytes). @@ -376,6 +421,58 @@ mod tests { assert_eq!(cmd.iter().fold(0u8, |a, &b| a.wrapping_add(b)), 0); } + /// Byte-for-byte against the frame accepted by the real device + /// (HW:2.0 / SW:1.7): `af 07 03 05 04 00 00 a0 40 5e` set 5.0 A and the + /// device acknowledged it with `df 07 03 05 01 00 11`. + #[test] + fn build_set_cap_current_matches_device_accepted_frame() { + assert_eq!( + build_set_cap_current_cmd(5.0), + vec![0xAF, 0x07, 0x03, 0x05, 0x04, 0x00, 0x00, 0xA0, 0x40, 0x5E] + ); + assert_eq!( + build_set_cap_current_cmd(3.3), + vec![0xAF, 0x07, 0x03, 0x05, 0x04, 0x33, 0x33, 0x53, 0x40, 0x45] + ); + } + + #[test] + fn cap_current_is_clamped_to_device_range() { + let hi = build_set_cap_current_cmd(99.0); + assert_eq!(f32::from_le_bytes([hi[5], hi[6], hi[7], hi[8]]), CAP_CURRENT_MAX_A); + let lo = build_set_cap_current_cmd(-1.0); + assert_eq!(f32::from_le_bytes([lo[5], lo[6], lo[7], lo[8]]), 0.0); + assert_eq!(hi.iter().fold(0u8, |a, &b| a.wrapping_add(b)), 0); + } + + #[test] + fn cap_current_read_command_checksum() { + assert_eq!(CMD_GET_CAP_CURRENT, [0xAF, 0x07, 0x03, 0x0A, 0x00, 0x3D]); + assert_eq!( + CMD_GET_CAP_CURRENT.iter().fold(0u8, |a, &b| a.wrapping_add(b)), + 0 + ); + } + + /// Real response captured from the device with its current set to 5000 mA. + /// Note the device quantises to milliamps, so 5.0 comes back as 5.0000010 — + /// readback must not be compared for exact equality. + #[test] + fn parse_cap_current_from_device_response() { + let resp = [0xDF, 0x07, 0x03, 0x0A, 0x04, 0x02, 0x00, 0xA0, 0x40, 0x27]; + let amps = parse_cap_current(&resp).expect("should parse"); + assert!((amps - 5.0).abs() < 1e-4, "got {amps}"); + } + + #[test] + fn parse_cap_current_rejects_other_packets() { + // Status packet header — must not be mistaken for a CAP current reply. + assert!(parse_cap_current(&[0xDF, 0x07, 0x03, 0x08, 0x16, 0, 0, 0, 0, 0]).is_none()); + // Acknowledgement of the set command, not the read reply. + assert!(parse_cap_current(&[0xDF, 0x07, 0x03, 0x05, 0x01, 0x00, 0x11]).is_none()); + assert!(parse_cap_current(&[]).is_none()); + } + #[test] fn build_mode_cmd_checksum() { let cmd = build_mode_cmd(Mode::CC);