RWTH-EBC / FiLiP

FIWARE Library for Python (FiLiP) to work with FIWARE API
BSD 3-Clause "New" or "Revised" License
23 stars 13 forks source link

fix: ngsi v2 tutorials are deprecated (#255) #259

Closed WaltherTrgovac closed 6 months ago

WaltherTrgovac commented 8 months ago

As already mentioned in the issue, the pydantic method parse_file_as is causing most problems. In the commit 'attempt' I have tried to fix it, but it still isn't working. If someone could check whether the implementation is correct, that would be nice. I will also just try to get all the tutorials to work for now with this pull request and then later reformat the rest...

Maghnie commented 8 months ago

Did you try the suggested change from https://github.com/RWTH-EBC/FiLiP/issues/255#issuecomment-1991304370 ?

Maybe validate_json instead of validate_python would work

However, not sure if that would fix the SERVICE_PATH error

WaltherTrgovac commented 7 months ago

I don't think that parsing is the problem anymore in exercise 5, but I can't get pass the error requests.exceptions.HTTPError: 500 Server Error: Internal Server Error for url: http://localhost:4041/iot/devices. I am missing some knowledge about HTTP Errors and can't debug it...

djs0109 commented 7 months ago

@WaltherTrgovac thanks for the effort! Your implementation looks good to me so far. I fix the bug you mention. The problem was, that a service group with cbHost="None" was created in e4, which is illegal regarding the data type of cbHost. Now, if you delete this service group manually via API, and redo e4 and then e5, you should be able to complete them

WaltherTrgovac commented 7 months ago

As of right now, exercise 5 and 6 are producing following error messages: requests.exceptions.HTTPError: 404 Client Error: Not Found for url: http://localhost:1026/v2/entities/urn:ngsi-ld:Heater:001/attrs?type=Heater . Interestingly, there is one test failing in test_ngsi_v2_cb.py. The name of the test is test_command_with_mqtt. This test also produces the same error (with different url) because it is almost equivalent code as in exercise 5 and 6. The same error (with different url) appears in e_08_ngsi_v2_iota_paho_mqtt.py and e_09_ngsi_v2_iota_filip_mqtt.py (after fixing other deprecated methods). I am pretty sure that after solving this problem all exercises will function properly.

WaltherTrgovac commented 7 months ago

Summary of this long merge request... Most changes are just formatting to keep the consistency throughout the whole tutorials.

Changes

In general, these are the changes:

For each exercise the changes are made:

  1. X-axis has been changed to show hours instead of minutes for better readability. time.sleep() has been reduced (controversial)
  2. Print format has been changed for better readability.
  3. Formatting
  4. X-axis has been changed to show hours instead of minutes for better readability. time.sleep() has been reduced (controversial). Each plot now has a title which increases understandability.
  5. Same as exercise 4
  6. Same as exercise 4
  7. Formatting

What has not been fixed

Exercise 5 and exercise 6 still not work properly due to the on command method (not sure whether the problem is actually here, but I am unable to fix it). This also causes weird plotting results on the x-axis which probably can not be fixed until this problem is resolved.

About the time.sleep() method

time.sleep() method is causing long execution times for the exercises. I understand that this function has to be there due to latency, but the problem is following: Let us assume that a person actually does exercises as a learning experience. It is very unlikely that the solution that the person writes is correct on the first execution. This causes that the person has to wait for tremendous amount of time in worst case for plotting to appear. I don't know whether a proper lower bound for waiting can be found but that would be my suggestion.

WaltherTrgovac commented 6 months ago

@WaltherTrgovac one last request for this PR. Could revise e6 as I made here? cf1659c I think you have reverted these changes somehow, but it should be easy to fix

It's done