Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create FillRateBasedControlTUNES #83

Merged
merged 21 commits into from
Jan 23, 2025

Conversation

victorgarcia98
Copy link
Collaborator

This PR includes the following key features:

  • Message handling happens in tasks that run asynchronously, e.g. handle_usage_forecast -> send_usage_forecast.
  • Send Storage fill_level to FM.
  • Send Actuator fill_rate to FM which relays the message to the sensor of the active operation mode (NES or THP charging).
  • Background recurrent task to trigger a schedule.
  • Send conversion efficiencies of THP and NES to FM .
  • Send FRBC.UsageForecast.
  • Send FRBC.FillLevelTargetPorfile to the sensors soc_maxima and soc_minima.
  • POC for wrapping a S2 Message with an envelope to include metadata.

…C control type to work only with TUNES RM (for now).

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98 victorgarcia98 self-assigned this Aug 7, 2024
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98 victorgarcia98 requested a review from Flix6x August 9, 2024 10:29
@victorgarcia98 victorgarcia98 marked this pull request as ready for review August 9, 2024 10:29
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

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

Start of a review, for our session tomorrow.

Comment on lines +93 to +97
task = asyncio.create_task(self.send_storage_status(message))
self.background_tasks.add(
task
) # important to avoid a task disappearing mid-execution.
task.add_done_callback(self.background_tasks.discard)
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self: candidate code block for refactoring.

Comment on lines +147 to +162
async def send_storage_status(self, status: FRBCStorageStatus):
raise NotImplementedError()

async def send_actuator_status(self, status: FRBCActuatorStatus):
raise NotImplementedError()

async def send_leakage_behaviour(self, leakage_behaviour: FRBCLeakageBehaviour):
raise NotImplementedError()

async def send_usage_forecast(self, usage_forecast: FRBCUsageForecast):
raise NotImplementedError()

async def send_fill_level_target_profile(
self, fill_level_target_profile: FRBCFillLevelTargetProfile
):
raise NotImplementedError()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it correct to say these are meant to be implemented in subclasses? (If so, note to self: add inline comments.)

"provides_power_measurement_types": ["ELECTRIC.POWER.3_PHASE_SYMMETRIC"],
"message_type": "ResourceManagerDetails",
},
"metadata": {"dt": "2023-01-01T00:00:00"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Pydantic is converting this to the datetime specified in the S2Wrapper class, right? I recommend adding a timezone offset explicitly, to avoid ambiguity. We can probably tell Pydantic that. Maybe also that this should be an ISO string.

Comment on lines 257 to 272
@pytest.mark.asyncio
async def test_trigger_schedule(cem_in_frbc_control_type):
cem, fm_client = await cem_in_frbc_control_type
# frbc = cem._control_types_handlers[cem.control_type]

tasks = get_pending_tasks()

assert tasks["trigger_schedule_task"]._state == "PENDING"
await cem.close()

tasks = get_pending_tasks()

assert tasks["trigger_schedule_task"]._state == "PENDING"

await cem.close()
get_pending_tasks()
Copy link
Contributor

Choose a reason for hiding this comment

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

This test I don't understand.

Copy link
Contributor

Choose a reason for hiding this comment

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

This test checks whether the task starts when the FRBC control type is selected.

Perhaps the task should actually stop when the CEM closes. But at least the RM doesn't get messages anymore after the CEM closes.

assert second_call[key] == second_call_expected[key]

await cem.close()
get_pending_tasks()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? Does this still test something?

Copy link
Contributor

Choose a reason for hiding this comment

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

This might not be needed. Or it is, and it cleans up tasks. Let's check.

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…TUNES/send-system-description-to-fm

# Conflicts:
#	README.rst
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

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

Merging so we can move on with python-s2 compatibility.

@Flix6x Flix6x merged commit b88c8d6 into main Jan 23, 2025
6 checks passed
@Flix6x Flix6x deleted the feature/TUNES/send-system-description-to-fm branch January 23, 2025 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants