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

ADC: replace ioctl with uorb message #14087

Merged
merged 43 commits into from Mar 20, 2020

Conversation

SalimTerryLi
Copy link
Contributor

This is spitted from #13833 .

This PR focus on replace ioctls with uORB message. Some existing modules/drivers are affected.

@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from 371ed98 to c6f9b61 Compare February 3, 2020 06:56
@SalimTerryLi
Copy link
Contributor Author

SalimTerryLi commented Feb 3, 2020

Device ID is set but not used. All logic kept same as before. This should be safe to merge.

@SalimTerryLi
Copy link
Contributor Author

SalimTerryLi commented Feb 3, 2020

There are two things to be checked:

  1. whether old ioctl message manages channel_id field correctly? I mean that adc_msg[A].am_channel == A should be true if that channel is valid.
  2. Do all boards have the same 3v3 ADC vref? If not, there should be some macro defined cases in ADC.cpp.

Confirmed. There is no problem.

@SalimTerryLi SalimTerryLi marked this pull request as ready for review February 3, 2020 07:16
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from c6f9b61 to 1c9451e Compare February 6, 2020 05:12
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Good rework, thanks.
What is the testing you have done?

src/drivers/adc/ADC.cpp Outdated Show resolved Hide resolved
src/modules/battery_status/analog_battery.cpp Show resolved Hide resolved
src/modules/battery_status/analog_battery.cpp Show resolved Hide resolved
src/drivers/rc_input/RCInput.cpp Outdated Show resolved Hide resolved
src/modules/battery_status/battery_status.cpp Outdated Show resolved Hide resolved
msg/adc_report.msg Outdated Show resolved Hide resolved
src/modules/battery_status/battery_status.cpp Outdated Show resolved Hide resolved
src/modules/battery_status/battery_status.cpp Outdated Show resolved Hide resolved
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from 1126845 to b445912 Compare February 7, 2020 15:44
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from 9807abc to 9188c4a Compare February 7, 2020 17:43
@SalimTerryLi
Copy link
Contributor Author

SalimTerryLi commented Feb 8, 2020

This is the last reference to that parameter. But I cannot find anything in the code which provides ADC specification such as sample precision and voltage reference. 0.0008 mostly indicate that the board is using 3v3 as vref with 2^12 bits precision but I cannot find anything related in code.
@bkueng Sorry but I'm unfamiliar with these devices. Any help is welcome.

@SalimTerryLi
Copy link
Contributor Author

Good rework, thanks.
What is the testing you have done?

Currently I have no hardware to test. This pr is diverged from a previous one so my test code on my platform could not be uploaded. If needed, I could commit them temporarily and then revert them for a clean PR.

@bkueng
Copy link
Member

bkueng commented Feb 10, 2020

This is the last reference to that parameter. But I cannot find anything in the code which provides ADC specification such as sample precision and voltage reference. 0.0008 mostly indicate that the board is using 3v3 as vref with 2^12 bits precision but I cannot find anything related in code.
@bkueng Sorry but I'm unfamiliar with these devices. Any help is welcome.

They should not have been added to that board, you can just remove them from there (the default should be used).

@SalimTerryLi
Copy link
Contributor Author

This is the last reference to that parameter. But I cannot find anything in the code which provides ADC specification such as sample precision and voltage reference. 0.0008 mostly indicate that the board is using 3v3 as vref with 2^12 bits precision but I cannot find anything related in code.
@bkueng Sorry but I'm unfamiliar with these devices. Any help is welcome.

They should not have been added to that board, you can just remove them from there (the default should be used).

Acknowledged. I'm contacting dagar to get ADC reference voltage properly provided for each board.

src/drivers/adc/ADC.cpp Outdated Show resolved Hide resolved
src/drivers/rc_input/RCInput.cpp Show resolved Hide resolved
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from 88e63e3 to a14a1d0 Compare February 21, 2020 15:52
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from f6eae80 to da0a336 Compare February 22, 2020 07:58
@SalimTerryLi
Copy link
Contributor Author

SalimTerryLi commented Feb 22, 2020

Solved conflict with master 4c1adc0
Probably caused by in-formatted white spaces that make format doesn't deal with.

@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch 3 times, most recently from c4d7c6b to 882acd8 Compare February 22, 2020 08:13
@SalimTerryLi
Copy link
Contributor Author

It's ready. @dagar
If someone could provide a test on the real hardware to prove its correctness would be better. Sorry for that I do not have any device to do such a test. If analog battery is monitored correctly, then this is safe to merge.

src/drivers/adc/ADC.cpp Outdated Show resolved Hide resolved
src/drivers/rc_input/RCInput.cpp Show resolved Hide resolved
SalimTerryLi and others added 20 commits March 20, 2020 17:09
1. remove cdev implement and use uORB publication
2. publish all channels instead of only battery voltage moniting channel
3. correct vref to 3.0f
4. temporarily disable test function
This would imporve the performance
Critical bug fix 1

Co-Authored-By: David Sidrane <David.Sidrane@Nscdg.com>
Critical bug fix 2

Co-Authored-By: David Sidrane <David.Sidrane@Nscdg.com>
Co-Authored-By: Beat Küng <beat-kueng@gmx.net>
Critical bug fix

Co-Authored-By: Beat Küng <beat-kueng@gmx.net>
analog differential pressure sensor reads from uORB(sensors.cpp)
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from ba94c6a to b431f72 Compare March 20, 2020 09:17
@bkueng bkueng merged commit dc8e775 into PX4:master Mar 20, 2020
Test Flights automation moved this from Ready for testing to Done Mar 20, 2020
@SalimTerryLi SalimTerryLi deleted the pr-adc_replace_ioctl_with_uorb-improve branch April 24, 2023 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Test Flights
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

9 participants