TechnionYP5777 / Smartcity-Smarthouse

Smartcity-Smarthouse
11 stars 5 forks source link

Mutual thinking: integrating both parts of the system #73

Closed SharonKL closed 7 years ago

SharonKL commented 7 years ago

The system logic is divided into two: Sensors Handling and Applications Handling. Each part has been developed seperatly in the past week and a half and has reaching impressive results; however, integration can be done without some serious refactoring to one, if not both, parts.

In this discussion I hope we can find a solution to this problem. The different aspects we will need to focus on, as I see it, are:

To achieve these goals we will have to define the exact data structure we use, the exact functions we implement that multiple parts use and make sure we all agree on the procedure needed to add or receive data.

@yossigil I know this issue has many assignees and labels, but this is an urgent discussion that concerns all of us and will impact many, if not all, parts of the code base.

update

After long and convoluted discussion we birthed the following conclusion. sidenote: Lets never do this again :skull: :sweat_smile:

The conclusions:

SharonKL commented 7 years ago

I will start with the basics - I think the databases need to be stored in the MainSystem class, as a Map<String, Table<String, Object>> field, mapping from sensors' ids to their database.

When the SensorsHandler receives a register message, it initializes a new database for the sensors. When the SensorsHandler receives an update message, it acesses an already initialized database and adds a new entry.

SharonKL commented 7 years ago

About the listeners, I think the table (being the data structure) should allow them to be added and invoke them.

When we do that, I think the SensorInfo class is redundant, databases can be all organized in the main system, and listeners will be attached to the datastructure, Table. @EliaTraore @inbalzukerman @ylevv

SharonKL commented 7 years ago

@EliaTraore Regarding the SamplesTable, it would be cleaner to simply define a Table<String, Object> instead of extending a class and not adding any functionallity, imo.

SharonKL commented 7 years ago

@EliaTraore @RonGatenio @roy-shchory Do you want the applications to receive a single entry, i.e. Map<String, Object>, or a whole table when listening to sensors?

EliaTraore commented 7 years ago

@SharonKL , imo if the database is stored in the MainSystem, it should be the one to initialize a new database and manage the access into it.
And regarding the question brought up in the issue, I can elaborate from the application handling perspective. In the current implementation -

SharonKL commented 7 years ago

Regarding the usage of commercialName - currently each sensor is identified by its ID. In the future we will implement a full 'type' system where each sensor might have several types and applications request sensors of specific types. But for now, especially for the prototype, it would be easier to simply request a sensors based on its ID.

EliaTraore commented 7 years ago

do you mean on a table level? because in that case i do not agree. I think the idea of a Databasehandler (that could be implemented by MainSytem btw) offers a needed abstraction.

SharonKL commented 7 years ago

I am removing @roy-shchory & @ylevv from the assigness as they work mostly on the application and sensor side of the system, respectively.

EliaTraore commented 7 years ago

DatabaseHandler holds all the tables :smile: and that was exactly my thought - this is where all the data is saved, accessed by the API defined in the DatabaseHandlerAPI and whatever the sensors want to add to that API. And turning it into a class sounds like a good idea (I don't see a reason to have multiple implementations of that)
As for ID vs commercialName, I actually think using commercialName is less effort. the ID is randomly generated in the system at registration (or so it should be, never got to reading that part of the implementation :sweat_smile: ), meaning that in order to make them known to the application we will have to add methods that retrieve all the IDs and gives them to the apps or something of the sort. On the other hand, to use the commerical name we will only have to add a field to the registeration msg of the sensors, and have the databaseHandler have a mapping from the ID to its commercialName. The latter requires less code (considering that the DatabaseHandler haven't been written yet) and have more code that can be used in the future. After those are added to the system, the only thing we will need to do is to choose the names and hardcode them into the apps and sensors we offer. If you are willing to take care of the field adding to the msg, i'll be more then happy to write the DatabaseHandler and complete the missing piece :)

inbalzukerman commented 7 years ago

I agree with @SharonKL - imo, the databases should be stored in the MainSystem class. About the listeners- it might be a good idea to add this functionality to the Table, although I thought at first it might have been a better idea to connect a listener with a sensor, since listeners from the applications, fr my understanding, are waiting for information from a sensor.

If what you guys meant ( @EliaTraore , @SharonKL ) by talking about the DatabaseHandler was to a class which represents the entire database- why not using the MainSystem class for that? I thought this was the idea of this class πŸ˜•

About:

The listeners expect to receive the whole table, from which they will extract the needed entries.

I thought the listeners only expect to receive the newest update, since for my understanding the application is always getting the newest information and process it, this is part of the reasons not all the required methods were implemented - it will be fixed asap, after conclusions on this matter will be achieved.

In conclusion, imo -

BTW- @SharonKL - Fyi, many labels is a good sign, it makes issues more specifically defined and make it easier to find if needed πŸ˜„

SharonKL commented 7 years ago

Sensors

@EliaTraore ragarding the names vs IDs - in the meeting when we talked about the sensors it was decided that sensors will come with pre-assigned IDs - just like MAC addresses. There are several advantages for this option, the main one can be illustrated using an example:

If an ID is given to a sensor upon registration, a problem might arise if the sensor shuts down unexpectedly. When the sensor reboots, it will have to request a new ID, and all the previous information will be lost. If we use predefined IDs instead, this will not be a problem since the ID of sensors remains the same between reboots, and the system will know to associate the information gathered before the shut down with the correct sensor.

Now, why do applications need to know the IDs of the sensors, and cannot simply ask for the type of information they want? Consider we want to place two butt sensors in our smart house - how would the application be able to identify which sensor is which? If the application simply requests the system to give it information from "some butt sensor", it will receive information gathered from both sensors and will mix up the data. Instead, my suggestion in the meeting was to have the following protocol - the application asks the system:

What sensors of type "butt sensor" does this house contain?

The system will reply with something like:

This house contains two sensors of type "butt sensor", their IDs are: 00:11:22:33:44:55 and 11:11:22:33:55:88

From now on the application has full control over what data to receive, and from which sensor. It will also be able to ask for data from a specific sensor.

System Design

@inbalzukerman @EliaTraore I agree with both of you on several things, and disagree with you and @SharonKL on others.

After many hours without sleep, I think it might be a good idea to have a class that will handle all the databases, simply to split the logic and make each part of the system responsible only for one thing. This class, lets name it for now DatabasesHandler, will:

In such case, where we split the logic, the MainSystem (which imo can be renamed to System, there is only one system in this project) will hold references to the three parts: the SensorsHandler. AppsHandler and DatabasesHandler. There is no reason for a class such as DatabasesHandlerAPI to exist, the API is simply the functions exposed by the DatabasesHandler - this API will allow the SensorsHandler to add new content, and allow the AppsHandler to add new listeners and poll data.

tl;dr

Sorry about the megila I wrote

EliaTraore commented 7 years ago

Imma point fingers and say that sending the whole table is to blame on the application team (shoutout to @RonGatenio @roy-shchory) , I'm only the lobbyist of their needs :grin: And as for the listeners, wasn't the table intended for more than the single purpose of being a sensor database? If so, it will make it less generic to have listeners in it....

EliaTraore commented 7 years ago

Now, why do applications need to know the IDs of the sensors, and cannot simply ask for the type of information they want? Consider we want to place two butt sensors in our smart house - how would the application be able to identify which sensor is which? If the application simply requests the system to give it information from "some butt sensor", it will receive information gathered from both sensors and will mix up the data.

The problem with that is, as we previously discussed in multiple platforms, that a sensor will have to define its type. So in reality you won't have a "butt sensor" type, you will have "ass sensor", "hiney sensor" and a "iButtocks sensor" all trying to describe the same thing. So our type list will not converge a nice small type list, but instead have aliases and duplications all over.

BUT, if the sensors ID are their macs, that makes the job even easier. When the DatabaseHandler is written it will have a method that adds a sensor with no type in which it will register its type to be its id. The MAC is simulated with the rest of the sensor so its under our control, so the applications could just request the comname==mac and our current code will be usable as it is.


as for the DatabaseHandler API interface, I suggest we will keep it at the time being. Add to it whatever the sensors require, and the implementer of the interface will be responsible to refactor the code after the class's completion and remove the interface. I will update issue #52 about the goals of the DatabaseHandler the we already got to agreement about.

EliaTraore commented 7 years ago

In order to speed up the process, @SharonKL had a real-time conversation (:hushed:). Below are our conclusions. If by evening time there will be no objection, the issue will be closed and executed.

The conclusions:

inbalzukerman commented 7 years ago

@EliaTraore , @SharonKL - I agree with some of your conclusions, they sound reasonable. Especially I liked the idea of implementing ListenableTable to keep the generic Table class as generic (therefore useful) as possible.

But I want to point out some questions -

Talking about how generic the Table class is- it was implemented especially to avoid determining the types the sensors will send and the applications will receive. This was the point of implementing it using Map<object, Object>... I do understand why it is easier to assume that:

Table<String,String> will be used through-out the project. Those are the only tables the sensors and the applications will interactive with.

But this is an assumption we were trying to avoid from the beginning. What is the sudden change?

Another question: Why does handling the data (namely - the required functionality of DatabaseHandler ) should be separated from System class? managing the database can, imo, easily fit in the System class since this is the defined responsibility of it. What benefit will we gain from creating the DatabaseHandler class and then let the System implement methods which will only call DatabaseHandler's methods? with no extra functionality? (This is basically the reason @SharonKL thought SensorInfo was redundant at first)

SharonKL commented 7 years ago

Table's Generictness

The fact we chose to use Table<String, String> to represent the information doesn't make the Table class any less generic. When we will use this class to store user information (adress, age, etc...) we can use something like Table<String, UserInfo>. The decission to use Table<String, String> was made because the system receives JSon objects that contain strings as keys and values. The applications will be able to receive these strings and convert them as they see fit (to integer for example).

Data Handling

The main objective and responsibility of the system is to be the bridge connecting the three parts of our project - sensors, applications and databases. For each part of the project we create a handler (which is part of the system core) that is responsible for communicating with that part. The MainSystem holds all the handlers and allows them to run. This is simply to make seperation between the different logics of our project - sensors and applications being independant programs, handlers being able to cmmunicate with them, databases to store data, and one system to control and oversee everything.

inbalzukerman commented 7 years ago

On a side note, there is no reason for Table<L extends Object, R extends Object> in the implementation of Table, it is equivalent to Table<L, R>.

Thanks for the code-review, noticed πŸ‘

EliaTraore commented 7 years ago

mmm... Hope we better communicate next time :sweat: :no_mouth: anyway,