Improve the HEMS tutorial for production use, asynchronous ingestion, cleanup, and interrupted-run recovery. - #222
Merged
Merged
Conversation
…be admin: account-admin suffices - improve docs and function names as all assets are in account Signed-off-by: Nicolas Höning <nicolas@seita.nl>
… on circumventing rate limiting as well Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
… tests Signed-off-by: Nicolas Höning <nicolas@seita.nl>
…not the strucure Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Coverage Report for CI Build 32385414968Coverage increased (+0.08%) to 96.658%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
…; add some helpful prints in the startup Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
… in graph Signed-off-by: Nicolas Höning <nicolas@seita.nl>
HEMS cleanup relies on depth zero to select only top-level assets, but the client discarded that value as falsy. This could broaden cleanup queries and make nested name collisions unsafe. Preserve zero-valued asset query parameters and expose the server's source and event-time filters for sensor data deletion. Add regression coverage for both request payloads. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Interrupted asset creation, partial data wipes, and legacy site names could leave users with unsafe or destructive recovery paths. Global name lookups could also select assets outside the intended community or account. Make structure repair idempotent, preserve existing IDs, and offer explicit keep, rename, recreate, continue-wipe, and exit choices where applicable. Validate stored workflow state, scope asset and sensor resolution, retain canonical ingestion jobs, and document the recovery behavior. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
FlexMeasures 0.33 returns ingestion IDs under job_id and expands asset and sensor queries to include roots or descendants. The HEMS recovery flow treated these valid responses as missing or ambiguous, preventing clean runs and incomplete-structure repair. Accept both job field names, deduplicate repeated roots by ID, and constrain sensor matches to exact owners while retaining genuine ambiguity checks. Align deletion confirmation and recovery documentation, remove the obsolete global lookup helper, and add regression coverage. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
BelhsanHmida
approved these changes
Aug 18, 2026
BelhsanHmida
left a comment
BelhsanHmida
left a comment
Contributor
There was a problem hiding this comment.
I completed a final review. looks good to me now. i pushed some follow-ups
The follow-up fixes make cleanup respect depth=0, add filtered sensor-data deletion, provide explicit and safe recovery choices for incomplete setups and interrupted wipes, preserve existing IDs when requested, scope asset and sensor lookups correctly, and support both FlexMeasures 0.33 job_id responses and newer job responses.
I manually verified the complete workflow on FlexMeasures 0.33, including resume, wipe recovery, incomplete-structure repair, legacy-name choices, scoped lookup, recreation cancellation, and cleanup. The full automated suite also passes: 220 tests, Black, Flake8, compilation, and git diff --check.
I found no remaining blockers. From my review, the PR is ready.
Building A asset graph:
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
The pre-commit workflow reordered the HEMS workflow test imports, causing CI to fail because the generated formatting diff was not committed. Apply the deterministic import order so the repository remains unchanged when the full pre-commit suite runs. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…olar by delivered PV instead of available PV, so curtailed energy no longer incorrectly lowers the KPI. Feed-in and curtailment are also reported separately. Signed-off-by: Nicolas Höning <nicolas@seita.nl>
…es/flexmeasures-client into feat/hems-script-improvements
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
WORKFLOW_VERSION stayed at 1 while the PV commits changed the asset
structure: they added the solar-feed-in and solar-curtailment sensors,
switched the site flex-context to inflexible-consumption/production, and
added dashboard panels. get_workflow_state() accepts any marker whose
version equals WORKFLOW_VERSION, so a setup created before those commits
still validated as current.
That left every existing setup unusable. Asset setup only runs from the
create, recreate and untracked-repair paths; resume and wipe both skip
it, so the new sensors were never created. Report generation then looks
them up unconditionally and find_sensor_by_name_and_asset() raises rather
than skipping, so create_reports() failed with
LookupError: Sensor 'solar-feed-in' not found in asset 'PV 1'
which propagates out of main(). Every rerun hit the same wall. The wipe
path was worst: it reset the phase markers, re-ran the whole scheduling
simulation, and only then failed. The sole escape was recreation, which
deletes every asset, sensor, ID and all time-series data.
Bump WORKFLOW_VERSION to 2 and accept any marker at or below it, so an
older setup is recognised as upgradable rather than current. After the
existing prompts have been resolved, run the idempotent asset setup again
and re-record the marker.
Ordering matters: the upgrade runs last, so an interrupted wipe is still
recovered through its own prompt first, and a recreation has already
rebuilt the structure at the current version.
The upgrade is deliberately cheap. Asset and sensor IDs are preserved
because get_or_create_* reuse whatever exists. Sensor IDs are re-collected
from the server afterwards, so a later wipe also covers sensors the
upgrade added. The recorded site names and the status are kept, so an
interrupted wipe stays recoverable. Only report generation is re-run,
since the reports write the sensors an upgrade tends to add, while the
uploaded data, forecasts and schedules stay valid.
Markers from a newer version are still rejected, since this script cannot
know what they describe.
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
docs/HEMS.rst stated two different requirements. The environment section said the example requires FlexMeasures 1.0 or newer, while the setup steps below still said 0.33.0 or newer was enough for the data-preserving wipe. 1.0 is the requirement that matches the code. HEMS_setup.py gates on "1.0.0.dev0", and the floor is substantive rather than cosmetic: the site flex-context is written with inflexible-consumption and inflexible-production, which only exist from the 1.0 series. A reader who followed the 0.33 line would set up a server that rejects the flex-context on the first schedule. Remove the stale 0.33 bullet. The 1.0 statement already sits above, next to the install instructions, and the rate-limiting note further down also refers to v1.0. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
const.py was changed to default to toy-user@flexmeasures.io / toy-password, the account created by `flexmeasures add toy-account`. The setup instructions were not changed with it: docs/HEMS.rst and the NOTE comment in HEMS_setup.py still told the reader to create an account named "HEMS tutorial" with a hems-admin@example.com user. Following those instructions produced a server with an account the script never tries to log into. The script then failed at startup with "User with email 'hems-admin@example.com' does not exist", and nothing pointed at the shipped defaults as the cause. Align the instructions with the code rather than the other way around. The toy account is a single command instead of three, and it already grants the account-admin role the tutorial needs, so the quick path is now the documented one and needs no edit to const.py. Keep the custom-account route for readers who want their own account, now stated as the alternative it has become, and say explicitly that const.py has to be updated to match in that case. Also mention that the toy account brings its own demo assets (toy-building and its children). The tutorial neither uses nor deletes them: delete_hems_assets() only removes assets it knows by name, and collect_hems_structure_ids() only walks the community, market and weather-station trees. Verified by running the tutorial end to end against a toy account. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
BelhsanHmida
approved these changes
Aug 20, 2026
BelhsanHmida
left a comment
BelhsanHmida
left a comment
Contributor
There was a problem hiding this comment.
Verified the five commits statically and with a full tutorial run against FM 1.0.0rc5.
The 0.25 energy-cost factor checks out arithmetically on live data (17.007 kW × 0.09717 EUR/kWh × 0.25 = 0.41314, matching the reported value exactly). The EV soc-usage restructuring is accepted by the FM 1.0 scheduler and correctly models both commute legs. Feed-in / curtailment / self-consumption formulas are sign-consistent. Sensor creation is idempotent.
Pushed three commits: the workflow-version bump (existing setups were hitting an unrecoverable LookupError on solar-feed-in), plus two docs fixes.
BelhsanHmida
added a commit
that referenced
this pull request
Aug 25, 2026
Resolved three conflicts, all in favour of the API-based reporting, while keeping what #222 added on top: - reporters.py: keep the CLI probe deleted, keep the new solar-feed-in and solar-curtailment outputs - scheduling.py: keep the async run_community_aggregate, keep #222's return-value check and the ingestion wait before it - HEMS.rst: worker now listens on ingestion and reporting, and the CLI workaround section stays deleted #222 put the solar sensors on the PV asset while the rest of the self-consumption outputs sit on the site, so that report can no longer derive its asset. It now names the site, which holds both in its subtree. Also noted that report triggers share the rate limit with schedules and forecasts, and raised the documented server floor to 1.1.0. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improve the HEMS tutorial for production use, asynchronous ingestion, cleanup, and interrupted-run recovery.
Production issues addressed
Closes https://github.com/SeitaBV/ems/issues/249
Verification
Remaining limitation
Report generation still requires a FlexMeasures CLI connected to the server database. It should be separated from the API-driven setup until reporting APIs are available.