Iss #254 adding additional fields to lidar config

[kersting]

Added new fields to lidar_config table

1. logger_stated_device_datum_plane_height: Height of the datum plane as programmed into the logger
2. logger_stated_device_orientation_deg: Device orientation in degrees as programmed into the logger

Updated the following files

• iea43_wra_data_model.schema
• iea43_wra_dat_model_postgresssql.sql
• CHANGELOG.md
• floating_lidar_demo_iea43_wra_data_model.json

[stephenholleran]

Hey @kersting,

Thanks for having a go with this. I made just some minor enough comments.

This PR however is merging directly into master. Could you change it to merge into the dev branch?

Cheers,

[kersting]

@stephenholleran thank you for the review. I did my pull request on Sunday, it clearly was a mistake. I think I fixed everything as I have also moved the request to dev. Could you please do a last sanity check? Feel free to merge into dev if everything is good or you can just ping me here.

[stephenholleran]

254

Hi @kersting,

Thanks for the changes, it looks way better now.

I made some edits to the descriptions. Make sure you agree with them.

What do you think of adding in examples to the main demo file? https://github.com/IEA-Task-43/digital_wra_data_standard/blob/master/demo_data/iea43_wra_data_model.json

And while I was reviewing it I realised that the device datum plane height programmed into the device could be for a lidar or a sodar. However we have applied the logger_stated into the 'lidar_config' table. We might want to think about renaming this 'lidar_config' table to something else but that would be a breaking change. Though this is a bit confusing for someone setting up a sodar I think we can let this slide and follow up with a name change when we are doing other breaking changes.

@abohara it would be good to get your thoughts and review of this PR too?

Thanks,

[kersting]

@stephenholleran thank you for your comments and for editing the descriptions. I'm fine with your edits.

As far as adding to the main demo file, I changed the floating lidar one instead as the main demo file is focused in met mast.

Nice catch regarding the lidar/sodar situation. I'm not opposed to rename from lidar_config to rsd_config. However, I feel that sodars are more of a legacy device at this point so I don't need to rush. On the other hand, I think we need to make the break change so maybe we may want to bundle with other items that will cause a break change as well.

[stephenholleran]

far as adding to the main demo file, I changed the floating lidar one instead as the main demo file is focused in met mast.

Ah yes, good point.

Nice catch regarding the lidar/sodar situation. I'm not opposed to rename from lidar_config to rsd_config. However, I feel that sodars are more of a legacy device at this point so I don't need to rush. On the other hand, I think we need to make the break change so maybe we may want to bundle with other items that will cause a break change as well.

I think these changes can continue as they are but we should create another issue to change the name of the 'lidar_config' table to something like you suggest, 'rsd_config', and bundle it in with the other breaking changes we have lined up.

So, I am good with this PR. It would be good to get @abohara to cast his eye over it too before merging.

Thanks,

[kersting]

@stephenholleran I will wait for @abohara's opinion and review to open a new issue to rename the table to rsd_config and I can assign that to myself unless someone ones to take this one.

[stephenholleran]

Hi @abohara, Any chance you can take a look at this? This PR is updating the Schema so it would be great for you to review. Cheers,