Skip to content

Conversation

@hikinggrass
Copy link
Member

@hikinggrass hikinggrass commented Nov 22, 2023

This was accidentally sent all the time but is only valid if there is a transaction ongoing

Fix verified against OCTT tests
TC_L_01_CS
TC_L_02_CS
TC_L_03_CS
TC_L_05_CS
TC_L_06_CS
TC_L_07_CS
TC_L_08_CS
TC_L_10_CS
TC_L_15_CS
TC_L_18_CS

This was accidentally sent and is completely wrong here

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

const auto transaction_active = this->any_transaction_active(std::nullopt);
if (transaction_active) {
this->firmware_status = FirmwareStatusEnum::InstallScheduled;
Copy link
Contributor

Choose a reason for hiding this comment

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

I will test your solution. But as we are already sending InstallScheduled from our 'core', will that go well?

Anyway, will test, let you know!

Copy link
Member Author

Choose a reason for hiding this comment

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

That might be an issue, maybe we can use another method of determining if transactions are still running that doesn't involve this part of the code in libocpp

Copy link
Contributor

Choose a reason for hiding this comment

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

This is indeed failing L_15. Because InstallScheduled is already sent (from the 'core' application), it will send it again.

It is a bit strange that all firmware statusses are sent from 'core' and only one is sent from here isn't it?
But keeping a variable that stores the last sent status will probably do the trick as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, this isn't the proper place for it. I'll remove the commit from this PR and figure something out for L_15

@hikinggrass hikinggrass force-pushed the kh-fix-wrong-install-scheduled branch from 1ac13ce to 62922d6 Compare November 23, 2023 08:20
@hikinggrass hikinggrass requested a review from maaikez November 23, 2023 08:24
@hikinggrass hikinggrass merged commit 4cff8fe into main Nov 23, 2023
@hikinggrass hikinggrass deleted the kh-fix-wrong-install-scheduled branch November 23, 2023 08:29
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.

3 participants