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

PoC: Interface for Firebolt Privacy Module #332

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
57 changes: 57 additions & 0 deletions interfaces/IFireboltPrivacy.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* If not stated otherwise in this file or this component's LICENSE file the
* following copyright and licenses apply:
*
* Copyright 2022 Metrological
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once
#include "Module.h"

namespace WPEFramework {
namespace Exchange {

/* @json 1.0.0*/
struct EXTERNAL IFireboltPrivacy : virtual public Core::IUnknown {
enum { ID = ID_FIREBOLT_PRIVACY };

// @event
struct EXTERNAL INotification : virtual public Core::IUnknown {
enum { ID = ID_FIREBOLT_PRIVACY_NOTIFICATION };
~INotification() override = default;

// @brief Notifies that Allow Resume points value change
/* @alt:OnAllowResumePointsChanged */
virtual void OnAllowResumePointsChanged(const bool value) = 0;

};

~IFireboltPrivacy() override = default;

// Pushing notifications to interested sinks
virtual uint32_t Register(IFireboltPrivacy::INotification* sink) = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use Core::hresult as return type, if you want to know why give me a call
(and of course do that for all methods below)

virtual uint32_t Unregister(IFireboltPrivacy::INotification* sink) = 0;

// @property
// @brief Provides Current resume watch status
// @alt:AllowResumePoints
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why @alt, is this an existing interface?

Choose a reason for hiding this comment

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

The method names in Firebolt spec are camelcased. In our thunder, all methods are made small. In the Thunder version that RDK has, @text is having an issue and is not changing the method name. Hence I used alt to achieve the same.

virtual uint32_t AllowResumePoints(bool& allow /* @out */) const = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

As it mentions pointS what is the use case for this interface? Can there be multiple points?
If it it only for testing why not reuse another existing interface? Or is there really an existing use case?

Choose a reason for hiding this comment

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

In Firebolt, the property name is called AllowResumePoints along with 's' hence the name.


// @brief Set resume watch status
// @alt:SetAllowResumePoints
virtual uint32_t SetAllowResumePoints(const bool& value ) = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

assuming the above and this method are a setter and getter (knowing the use case is helpful :) ) why is one a property and the other a method? (at least they should have the correct name and signature)

Choose a reason for hiding this comment

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

I read in the Thunder documentation that the properties should not do any other work other than just changing the property value. But in Firebolt, the expectation is that it has to update the cloud components and have to send that value change event. Hence made the setter as method and getter doesn't need any extra operation so made that as a property.

};
}
}
5 changes: 4 additions & 1 deletion interfaces/Ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,10 @@ namespace Exchange {

ID_DNS_SERVER = RPC::IDS::ID_EXTERNAL_INTERFACE_OFFSET + 0x4E0,
ID_DNS_ZONE = ID_DNS_SERVER + 1,
ID_DNS_RECORD = ID_DNS_SERVER + 2
ID_DNS_RECORD = ID_DNS_SERVER + 2,

ID_FIREBOLT_PRIVACY = RPC::IDS::ID_EXTERNAL_INTERFACE_OFFSET + 0x4F0,
ID_FIREBOLT_PRIVACY_NOTIFICATION = ID_FIREBOLT_PRIVACY + 1
};
}
}
Loading